[PATCH 0/4] Add CMN PLL clock controller support for IPQ5424

[PATCH 0/4] Add CMN PLL clock controller support for IPQ5424

[Luo Jie]

The CMN PLL block of IPQ5424 is almost same as that of IPQ9574
which is currently supported by the driver. The only difference
is that the fixed output clocks to NSS and PPE from CMN PLL have
a different clock rate. In IPQ5424, the output clocks are supplied
to NSS at 300 MHZ and to PPE at 375 MHZ.

Two related clock identifiers NSS_300MHZ_CLK and PPE_375MHZ_CLK
are added for IPQ5424. The new table of output clocks is added
for the CMN PLL of IPQ5424, which is acquired from the device
according to the compatible.

Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
Luo Jie (4):
      dt-bindings: clock: qcom: Add CMN PLL support for IPQ5424 SoC
      clk: qcom: cmnpll: Add IPQ5424 SoC support
      arm64: dts: ipq5424: Add CMN PLL node
      arm64: dts: qcom: Update IPQ5424 xo_board to use fixed factor clock

 .../bindings/clock/qcom,ipq9574-cmn-pll.yaml       |  1 +
 arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts        | 23 +++++++++++++--
 arch/arm64/boot/dts/qcom/ipq5424.dtsi              | 27 ++++++++++++++++-
 drivers/clk/qcom/ipq-cmn-pll.c                     | 34 ++++++++++++++++++----
 include/dt-bindings/clock/qcom,ipq-cmn-pll.h       | 10 ++++++-
 5 files changed, 86 insertions(+), 9 deletions(-)
---
base-commit: 9388ec571cb1adba59d1cded2300eeb11827679c
change-id: 20250321-qcom_ipq5424_cmnpll-416f770f2f97

Best regards,
-- 
Luo Jie <quic_luoj@quicinc.com>

[PATCH 1/4] dt-bindings: clock: qcom: Add CMN PLL support for IPQ5424 SoC

[Luo Jie]

The CMN PLL block in the IPQ5424 SoC takes 48 MHZ as the reference
input clock. The output clocks are the same as IPQ9574 SoC, except
for the clock rate of output clocks to PPE and NSS.

Also, add macros for clock rates that are applicable specifically
only for IPQ5424.

Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
 .../devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml        |  1 +
 include/dt-bindings/clock/qcom,ipq-cmn-pll.h                   | 10 +++++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
index f869b3739be8..bbaf896ae908 100644
--- a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
@@ -25,6 +25,7 @@ properties:
   compatible:
     enum:
       - qcom,ipq9574-cmn-pll
+      - qcom,ipq5424-cmn-pll
 
   reg:
     maxItems: 1
diff --git a/include/dt-bindings/clock/qcom,ipq-cmn-pll.h b/include/dt-bindings/clock/qcom,ipq-cmn-pll.h
index 936e92b3b62c..e30d57001c38 100644
--- a/include/dt-bindings/clock/qcom,ipq-cmn-pll.h
+++ b/include/dt-bindings/clock/qcom,ipq-cmn-pll.h
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
 /*
- * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2024-2025 Qualcomm Innovation Center, Inc. All rights reserved.
  */
 
 #ifndef _DT_BINDINGS_CLK_QCOM_IPQ_CMN_PLL_H
@@ -19,4 +19,12 @@
 #define ETH1_50MHZ_CLK			7
 #define ETH2_50MHZ_CLK			8
 #define ETH_25MHZ_CLK			9
+
+/*
+ * The CMN PLL output clock rates that are specifically applicable for IPQ5424
+ * SoC. For IPQ5424, the other output clocks and their rates are same as IPQ9574.
+ */
+#define NSS_300MHZ_CLK			4
+#define PPE_375MHZ_CLK			5
+
 #endif

-- 
2.34.1

[PATCH 2/4] clk: qcom: cmnpll: Add IPQ5424 SoC support

[Luo Jie]

The CMN PLL in IPQ5424 SoC supplies the fixed clock to NSS at 300 MHZ
and to PPE at 375 MHZ. Other output clocks from CMN PLL on this SoC,
and their rates are same as IPQ9574.

Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
 drivers/clk/qcom/ipq-cmn-pll.c | 34 +++++++++++++++++++++++++++++-----
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/qcom/ipq-cmn-pll.c b/drivers/clk/qcom/ipq-cmn-pll.c
index 432d4c4b7aa6..ce5e83124c6d 100644
--- a/drivers/clk/qcom/ipq-cmn-pll.c
+++ b/drivers/clk/qcom/ipq-cmn-pll.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2024-2025 Qualcomm Innovation Center, Inc. All rights reserved.
  */
 
 /*
@@ -16,6 +16,10 @@
  * are supplied to GCC (24 MHZ as XO and 32 KHZ as sleep clock), and to PCS
  * with 31.25 MHZ.
  *
+ * On the IPQ5424 SoC, there is an output clock from CMN PLL to PPE at 375 MHZ,
+ * and an output clock to NSS at 300 MHZ. The other output clocks from CMN PLL
+ * on IPQ5424 are the same as IPQ9574.
+ *
  *               +---------+
  *               |   GCC   |
  *               +--+---+--+
@@ -115,6 +119,20 @@ static const struct cmn_pll_fixed_output_clk ipq9574_output_clks[] = {
 	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 */ },
+};
+
+static const struct cmn_pll_fixed_output_clk ipq5424_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_300MHZ_CLK, "nss-300mhz", 300000000UL),
+	CLK_PLL_OUTPUT(PPE_375MHZ_CLK, "ppe-375mhz", 375000000UL),
+	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 */ },
 };
 
 /*
@@ -297,7 +315,7 @@ static struct clk_hw *ipq_cmn_pll_clk_hw_register(struct platform_device *pdev)
 
 static int ipq_cmn_pll_register_clks(struct platform_device *pdev)
 {
-	const struct cmn_pll_fixed_output_clk *fixed_clk;
+	const struct cmn_pll_fixed_output_clk *p, *fixed_clk;
 	struct clk_hw_onecell_data *hw_data;
 	struct device *dev = &pdev->dev;
 	struct clk_hw *cmn_pll_hw;
@@ -305,8 +323,13 @@ static int ipq_cmn_pll_register_clks(struct platform_device *pdev)
 	struct clk_hw *hw;
 	int ret, i;
 
-	fixed_clk = ipq9574_output_clks;
-	num_clks = ARRAY_SIZE(ipq9574_output_clks);
+	fixed_clk = device_get_match_data(dev);
+	if (!fixed_clk)
+		return -EINVAL;
+
+	num_clks = 0;
+	for (p = fixed_clk; p->name; p++)
+		num_clks++;
 
 	hw_data = devm_kzalloc(dev, struct_size(hw_data, hws, num_clks + 1),
 			       GFP_KERNEL);
@@ -415,7 +438,8 @@ 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", },
+	{ .compatible = "qcom,ipq9574-cmn-pll", .data = &ipq9574_output_clks },
+	{ .compatible = "qcom,ipq5424-cmn-pll", .data = &ipq5424_output_clks },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, ipq_cmn_pll_clk_ids);

-- 
2.34.1

[PATCH 3/4] arm64: dts: ipq5424: Add CMN PLL node

[Luo Jie]

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

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

Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
 arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts | 16 +++++++++++++++-
 arch/arm64/boot/dts/qcom/ipq5424.dtsi       | 26 +++++++++++++++++++++++++-
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts b/arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts
index 0fd0ebe0251d..b4d1aa00c944 100644
--- a/arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts
+++ b/arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts
@@ -2,7 +2,7 @@
 /*
  * IPQ5424 RDP466 board device tree source
  *
- * Copyright (c) 2024 The Linux Foundation. All rights reserved.
+ * Copyright (c) 2024-2025 The Linux Foundation. All rights reserved.
  */
 
 /dts-v1/;
@@ -213,7 +213,21 @@ &usb3 {
 	status = "okay";
 };
 
+/*
+ * The bootstrap pins for the board select the XO clock frequency that
+ * supports 48 MHZ, 96 MHZ or 192 MHZ. This setting automatically
+ * enables the right dividers, to ensure the reference clock output
+ * from WiFi to the CMN PLL is 48 MHZ.
+ */
+&ref_48mhz_clk {
+	clock-div = <1>;
+	clock-mult = <1>;
+};
+
 &xo_board {
 	clock-frequency = <24000000>;
 };
 
+&xo_clk {
+	clock-frequency = <48000000>;
+};
diff --git a/arch/arm64/boot/dts/qcom/ipq5424.dtsi b/arch/arm64/boot/dts/qcom/ipq5424.dtsi
index 5d6ed2172b1b..844788990f43 100644
--- a/arch/arm64/boot/dts/qcom/ipq5424.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5424.dtsi
@@ -3,10 +3,11 @@
  * IPQ5424 device tree source
  *
  * Copyright (c) 2020-2021 The Linux Foundation. All rights reserved.
- * Copyright (c) 2022-2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2022-2025 Qualcomm Innovation Center, Inc. All rights reserved.
  */
 
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/clock/qcom,ipq-cmn-pll.h>
 #include <dt-bindings/clock/qcom,ipq5424-gcc.h>
 #include <dt-bindings/reset/qcom,ipq5424-gcc.h>
 #include <dt-bindings/gpio/gpio.h>
@@ -17,6 +18,12 @@ / {
 	interrupt-parent = <&intc>;
 
 	clocks {
+		ref_48mhz_clk: ref-48mhz-clk {
+			compatible = "fixed-factor-clock";
+			clocks = <&xo_clk>;
+			#clock-cells = <0>;
+		};
+
 		sleep_clk: sleep-clk {
 			compatible = "fixed-clock";
 			#clock-cells = <0>;
@@ -26,6 +33,11 @@ xo_board: xo-board-clk {
 			compatible = "fixed-clock";
 			#clock-cells = <0>;
 		};
+
+		xo_clk: xo-clk {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+		};
 	};
 
 	cpus: cpus {
@@ -157,6 +169,18 @@ soc@0 {
 		#size-cells = <2>;
 		ranges = <0 0 0 0 0x10 0>;
 
+		cmn_pll: clock-controller@9b000 {
+			compatible = "qcom,ipq5424-cmn-pll";
+			reg = <0 0x0009b000 0 0x800>;
+			clocks = <&ref_48mhz_clk>,
+				 <&gcc GCC_CMN_12GPLL_AHB_CLK>,
+				 <&gcc GCC_CMN_12GPLL_SYS_CLK>;
+			clock-names = "ref", "ahb", "sys";
+			#clock-cells = <1>;
+			assigned-clocks = <&cmn_pll CMN_PLL_CLK>;
+			assigned-clock-rates-u64 = /bits/ 64 <12000000000>;
+		};
+
 		efuse@a4000 {
 			compatible = "qcom,ipq5424-qfprom", "qcom,qfprom";
 			reg = <0 0x000a4000 0 0x741>;

-- 
2.34.1

[PATCH 4/4] arm64: dts: qcom: Update IPQ5424 xo_board to use fixed factor clock

[Luo Jie]

xo_board is fixed to 24 MHZ, which is routed from WiFi output clock
48 MHZ (also being the reference clock of CMN PLL) divided 2 by
analog block routing channel.

Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
 arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts | 7 ++++++-
 arch/arm64/boot/dts/qcom/ipq5424.dtsi       | 3 ++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts b/arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts
index b4d1aa00c944..e503b6677a3e 100644
--- a/arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts
+++ b/arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts
@@ -224,8 +224,13 @@ &ref_48mhz_clk {
 	clock-mult = <1>;
 };
 
+/*
+ * The frequency of xo_board is fixed to 24 MHZ, which is routed
+ * from WiFi output clock 48 MHZ divided by 2.
+ */
 &xo_board {
-	clock-frequency = <24000000>;
+	clock-div = <2>;
+	clock-mult = <1>;
 };
 
 &xo_clk {
diff --git a/arch/arm64/boot/dts/qcom/ipq5424.dtsi b/arch/arm64/boot/dts/qcom/ipq5424.dtsi
index 844788990f43..6974339c126f 100644
--- a/arch/arm64/boot/dts/qcom/ipq5424.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5424.dtsi
@@ -30,7 +30,8 @@ sleep_clk: sleep-clk {
 		};
 
 		xo_board: xo-board-clk {
-			compatible = "fixed-clock";
+			compatible = "fixed-factor-clock";
+			clocks = <&ref_48mhz_clk>;
 			#clock-cells = <0>;
 		};
 

-- 
2.34.1

Re: [PATCH 2/4] clk: qcom: cmnpll: Add IPQ5424 SoC support

[Stephen Boyd]

Quoting Luo Jie (2025-03-21 05:49:53)
> diff --git a/drivers/clk/qcom/ipq-cmn-pll.c b/drivers/clk/qcom/ipq-cmn-pll.c
> index 432d4c4b7aa6..ce5e83124c6d 100644
> --- a/drivers/clk/qcom/ipq-cmn-pll.c
> +++ b/drivers/clk/qcom/ipq-cmn-pll.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /*
> - * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2024-2025 Qualcomm Innovation Center, Inc. All rights reserved.
>   */
>  
>  /*
> @@ -16,6 +16,10 @@
>   * are supplied to GCC (24 MHZ as XO and 32 KHZ as sleep clock), and to PCS
>   * with 31.25 MHZ.
>   *
> + * On the IPQ5424 SoC, there is an output clock from CMN PLL to PPE at 375 MHZ,
> + * and an output clock to NSS at 300 MHZ. The other output clocks from CMN PLL
> + * on IPQ5424 are the same as IPQ9574.
> + *
>   *               +---------+
>   *               |   GCC   |
>   *               +--+---+--+
> @@ -115,6 +119,20 @@ static const struct cmn_pll_fixed_output_clk ipq9574_output_clks[] = {
>         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 */ },

Nitpick: Drop the comma here so nothing can come after the sentinel.

> +};
> +
> +static const struct cmn_pll_fixed_output_clk ipq5424_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_300MHZ_CLK, "nss-300mhz", 300000000UL),
> +       CLK_PLL_OUTPUT(PPE_375MHZ_CLK, "ppe-375mhz", 375000000UL),
> +       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 */ },

Nitpick: Drop the comma here so nothing can come after the sentinel.

>  };
>  
>  /*

Re: [PATCH 1/4] dt-bindings: clock: qcom: Add CMN PLL support for IPQ5424 SoC

[Konrad Dybcio]

On 3/21/25 1:49 PM, Luo Jie wrote:
> The CMN PLL block in the IPQ5424 SoC takes 48 MHZ as the reference
> input clock. The output clocks are the same as IPQ9574 SoC, except
> for the clock rate of output clocks to PPE and NSS.
> 
> Also, add macros for clock rates that are applicable specifically
> only for IPQ5424.
> 
> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
> ---
>  .../devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml        |  1 +
>  include/dt-bindings/clock/qcom,ipq-cmn-pll.h                   | 10 +++++++++-
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
> index f869b3739be8..bbaf896ae908 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
> @@ -25,6 +25,7 @@ properties:
>    compatible:
>      enum:
>        - qcom,ipq9574-cmn-pll
> +      - qcom,ipq5424-cmn-pll
>  
>    reg:
>      maxItems: 1
> diff --git a/include/dt-bindings/clock/qcom,ipq-cmn-pll.h b/include/dt-bindings/clock/qcom,ipq-cmn-pll.h
> index 936e92b3b62c..e30d57001c38 100644
> --- a/include/dt-bindings/clock/qcom,ipq-cmn-pll.h
> +++ b/include/dt-bindings/clock/qcom,ipq-cmn-pll.h
> @@ -1,6 +1,6 @@
>  /* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
>  /*
> - * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2024-2025 Qualcomm Innovation Center, Inc. All rights reserved.
>   */
>  
>  #ifndef _DT_BINDINGS_CLK_QCOM_IPQ_CMN_PLL_H
> @@ -19,4 +19,12 @@
>  #define ETH1_50MHZ_CLK			7
>  #define ETH2_50MHZ_CLK			8
>  #define ETH_25MHZ_CLK			9
> +
> +/*
> + * The CMN PLL output clock rates that are specifically applicable for IPQ5424
> + * SoC. For IPQ5424, the other output clocks and their rates are same as IPQ9574.
> + */
> +#define NSS_300MHZ_CLK			4
> +#define PPE_375MHZ_CLK			5

Not a huge fan of this, such differences are only relevant to the driver
part in my view - bindings only let a consumer reference a specific piece
of hardware

Konrad

Re: [PATCH 1/4] dt-bindings: clock: qcom: Add CMN PLL support for IPQ5424 SoC

[Konrad Dybcio]

On 3/25/25 12:57 AM, Konrad Dybcio wrote:
> On 3/21/25 1:49 PM, Luo Jie wrote:
>> The CMN PLL block in the IPQ5424 SoC takes 48 MHZ as the reference
>> input clock. The output clocks are the same as IPQ9574 SoC, except
>> for the clock rate of output clocks to PPE and NSS.
>>
>> Also, add macros for clock rates that are applicable specifically
>> only for IPQ5424.
>>
>> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
>> ---
>>  .../devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml        |  1 +
>>  include/dt-bindings/clock/qcom,ipq-cmn-pll.h                   | 10 +++++++++-
>>  2 files changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
>> index f869b3739be8..bbaf896ae908 100644
>> --- a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
>> +++ b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
>> @@ -25,6 +25,7 @@ properties:
>>    compatible:
>>      enum:
>>        - qcom,ipq9574-cmn-pll
>> +      - qcom,ipq5424-cmn-pll
>>  
>>    reg:
>>      maxItems: 1
>> diff --git a/include/dt-bindings/clock/qcom,ipq-cmn-pll.h b/include/dt-bindings/clock/qcom,ipq-cmn-pll.h
>> index 936e92b3b62c..e30d57001c38 100644
>> --- a/include/dt-bindings/clock/qcom,ipq-cmn-pll.h
>> +++ b/include/dt-bindings/clock/qcom,ipq-cmn-pll.h
>> @@ -1,6 +1,6 @@
>>  /* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
>>  /*
>> - * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
>> + * Copyright (c) 2024-2025 Qualcomm Innovation Center, Inc. All rights reserved.
>>   */
>>  
>>  #ifndef _DT_BINDINGS_CLK_QCOM_IPQ_CMN_PLL_H
>> @@ -19,4 +19,12 @@
>>  #define ETH1_50MHZ_CLK			7
>>  #define ETH2_50MHZ_CLK			8
>>  #define ETH_25MHZ_CLK			9
>> +
>> +/*
>> + * The CMN PLL output clock rates that are specifically applicable for IPQ5424
>> + * SoC. For IPQ5424, the other output clocks and their rates are same as IPQ9574.
>> + */
>> +#define NSS_300MHZ_CLK			4
>> +#define PPE_375MHZ_CLK			5
> 
> Not a huge fan of this, such differences are only relevant to the driver
> part in my view - bindings only let a consumer reference a specific piece
> of hardware

Oh I the bindings are stepping into the frequency department already,
hmm.. Then I suppose it's fine if the dt-bindings maintainers don't have any
concerns

Konrad

Re: [PATCH 1/4] dt-bindings: clock: qcom: Add CMN PLL support for IPQ5424 SoC

[Krzysztof Kozlowski]

On Fri, Mar 21, 2025 at 08:49:52PM +0800, Luo Jie wrote:
> The CMN PLL block in the IPQ5424 SoC takes 48 MHZ as the reference
> input clock. The output clocks are the same as IPQ9574 SoC, except
> for the clock rate of output clocks to PPE and NSS.
> 
> Also, add macros for clock rates that are applicable specifically
> only for IPQ5424.
> 
> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
> ---
>  .../devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml        |  1 +
>  include/dt-bindings/clock/qcom,ipq-cmn-pll.h                   | 10 +++++++++-
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
> index f869b3739be8..bbaf896ae908 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
> @@ -25,6 +25,7 @@ properties:
>    compatible:
>      enum:
>        - qcom,ipq9574-cmn-pll
> +      - qcom,ipq5424-cmn-pll

Same comments as before... keep the order.

>  
>    reg:
>      maxItems: 1
> diff --git a/include/dt-bindings/clock/qcom,ipq-cmn-pll.h b/include/dt-bindings/clock/qcom,ipq-cmn-pll.h
> index 936e92b3b62c..e30d57001c38 100644
> --- a/include/dt-bindings/clock/qcom,ipq-cmn-pll.h
> +++ b/include/dt-bindings/clock/qcom,ipq-cmn-pll.h
> @@ -1,6 +1,6 @@
>  /* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
>  /*
> - * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2024-2025 Qualcomm Innovation Center, Inc. All rights reserved.
>   */
>  
>  #ifndef _DT_BINDINGS_CLK_QCOM_IPQ_CMN_PLL_H
> @@ -19,4 +19,12 @@
>  #define ETH1_50MHZ_CLK			7
>  #define ETH2_50MHZ_CLK			8
>  #define ETH_25MHZ_CLK			9
> +
> +/*
> + * The CMN PLL output clock rates that are specifically applicable for IPQ5424
> + * SoC. For IPQ5424, the other output clocks and their rates are same as IPQ9574.

Just come with different header. Why was this called in generic way,
since we ask to name the headers based on compatible?

Best regards,
Krzysztof

Re: [PATCH 1/4] dt-bindings: clock: qcom: Add CMN PLL support for IPQ5424 SoC

[Krzysztof Kozlowski]

On Tue, Mar 25, 2025 at 12:59:49AM +0100, Konrad Dybcio wrote:
> On 3/25/25 12:57 AM, Konrad Dybcio wrote:
> > On 3/21/25 1:49 PM, Luo Jie wrote:
> >> The CMN PLL block in the IPQ5424 SoC takes 48 MHZ as the reference
> >> input clock. The output clocks are the same as IPQ9574 SoC, except
> >> for the clock rate of output clocks to PPE and NSS.
> >>
> >> Also, add macros for clock rates that are applicable specifically
> >> only for IPQ5424.
> >>
> >> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
> >> ---
> >>  .../devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml        |  1 +
> >>  include/dt-bindings/clock/qcom,ipq-cmn-pll.h                   | 10 +++++++++-
> >>  2 files changed, 10 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
> >> index f869b3739be8..bbaf896ae908 100644
> >> --- a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
> >> +++ b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
> >> @@ -25,6 +25,7 @@ properties:
> >>    compatible:
> >>      enum:
> >>        - qcom,ipq9574-cmn-pll
> >> +      - qcom,ipq5424-cmn-pll
> >>  
> >>    reg:
> >>      maxItems: 1
> >> diff --git a/include/dt-bindings/clock/qcom,ipq-cmn-pll.h b/include/dt-bindings/clock/qcom,ipq-cmn-pll.h
> >> index 936e92b3b62c..e30d57001c38 100644
> >> --- a/include/dt-bindings/clock/qcom,ipq-cmn-pll.h
> >> +++ b/include/dt-bindings/clock/qcom,ipq-cmn-pll.h
> >> @@ -1,6 +1,6 @@
> >>  /* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> >>  /*
> >> - * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
> >> + * Copyright (c) 2024-2025 Qualcomm Innovation Center, Inc. All rights reserved.
> >>   */
> >>  
> >>  #ifndef _DT_BINDINGS_CLK_QCOM_IPQ_CMN_PLL_H
> >> @@ -19,4 +19,12 @@
> >>  #define ETH1_50MHZ_CLK			7
> >>  #define ETH2_50MHZ_CLK			8
> >>  #define ETH_25MHZ_CLK			9
> >> +
> >> +/*
> >> + * The CMN PLL output clock rates that are specifically applicable for IPQ5424
> >> + * SoC. For IPQ5424, the other output clocks and their rates are same as IPQ9574.
> >> + */
> >> +#define NSS_300MHZ_CLK			4
> >> +#define PPE_375MHZ_CLK			5
> > 
> > Not a huge fan of this, such differences are only relevant to the driver
> > part in my view - bindings only let a consumer reference a specific piece
> > of hardware
> 
> Oh I the bindings are stepping into the frequency department already,
> hmm.. Then I suppose it's fine if the dt-bindings maintainers don't have any
> concerns


Nooooo, it was said these are output clocks, not rates. If these are
rates, then NAK.

Best regards,
Krzysztof

Re: [PATCH 2/4] clk: qcom: cmnpll: Add IPQ5424 SoC support

[Jie Luo]

On 3/25/2025 7:42 AM, Stephen Boyd wrote:
> Quoting Luo Jie (2025-03-21 05:49:53)
>> diff --git a/drivers/clk/qcom/ipq-cmn-pll.c b/drivers/clk/qcom/ipq-cmn-pll.c
>> index 432d4c4b7aa6..ce5e83124c6d 100644
>> --- a/drivers/clk/qcom/ipq-cmn-pll.c
>> +++ b/drivers/clk/qcom/ipq-cmn-pll.c
>> @@ -1,6 +1,6 @@
>>   // SPDX-License-Identifier: GPL-2.0-only
>>   /*
>> - * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
>> + * Copyright (c) 2024-2025 Qualcomm Innovation Center, Inc. All rights reserved.
>>    */
>>   
>>   /*
>> @@ -16,6 +16,10 @@
>>    * are supplied to GCC (24 MHZ as XO and 32 KHZ as sleep clock), and to PCS
>>    * with 31.25 MHZ.
>>    *
>> + * On the IPQ5424 SoC, there is an output clock from CMN PLL to PPE at 375 MHZ,
>> + * and an output clock to NSS at 300 MHZ. The other output clocks from CMN PLL
>> + * on IPQ5424 are the same as IPQ9574.
>> + *
>>    *               +---------+
>>    *               |   GCC   |
>>    *               +--+---+--+
>> @@ -115,6 +119,20 @@ static const struct cmn_pll_fixed_output_clk ipq9574_output_clks[] = {
>>          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 */ },
> 
> Nitpick: Drop the comma here so nothing can come after the sentinel.

Understand, I will remove it.

> 
>> +};
>> +
>> +static const struct cmn_pll_fixed_output_clk ipq5424_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_300MHZ_CLK, "nss-300mhz", 300000000UL),
>> +       CLK_PLL_OUTPUT(PPE_375MHZ_CLK, "ppe-375mhz", 375000000UL),
>> +       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 */ },
> 
> Nitpick: Drop the comma here so nothing can come after the sentinel.
> 

OK.

>>   };
>>   
>>   /*
> 

Re: [PATCH 1/4] dt-bindings: clock: qcom: Add CMN PLL support for IPQ5424 SoC

[Jie Luo]

On 3/25/2025 4:19 PM, Krzysztof Kozlowski wrote:
> On Fri, Mar 21, 2025 at 08:49:52PM +0800, Luo Jie wrote:
>> The CMN PLL block in the IPQ5424 SoC takes 48 MHZ as the reference
>> input clock. The output clocks are the same as IPQ9574 SoC, except
>> for the clock rate of output clocks to PPE and NSS.
>>
>> Also, add macros for clock rates that are applicable specifically
>> only for IPQ5424.
>>
>> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
>> ---
>>   .../devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml        |  1 +
>>   include/dt-bindings/clock/qcom,ipq-cmn-pll.h                   | 10 +++++++++-
>>   2 files changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
>> index f869b3739be8..bbaf896ae908 100644
>> --- a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
>> +++ b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
>> @@ -25,6 +25,7 @@ properties:
>>     compatible:
>>       enum:
>>         - qcom,ipq9574-cmn-pll
>> +      - qcom,ipq5424-cmn-pll
> 
> Same comments as before... keep the order.

OK, I will update to keep the compatible strings in alphanumerical
order.

> 
>>   
>>     reg:
>>       maxItems: 1
>> diff --git a/include/dt-bindings/clock/qcom,ipq-cmn-pll.h b/include/dt-bindings/clock/qcom,ipq-cmn-pll.h
>> index 936e92b3b62c..e30d57001c38 100644
>> --- a/include/dt-bindings/clock/qcom,ipq-cmn-pll.h
>> +++ b/include/dt-bindings/clock/qcom,ipq-cmn-pll.h
>> @@ -1,6 +1,6 @@
>>   /* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
>>   /*
>> - * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
>> + * Copyright (c) 2024-2025 Qualcomm Innovation Center, Inc. All rights reserved.
>>    */
>>   
>>   #ifndef _DT_BINDINGS_CLK_QCOM_IPQ_CMN_PLL_H
>> @@ -19,4 +19,12 @@
>>   #define ETH1_50MHZ_CLK			7
>>   #define ETH2_50MHZ_CLK			8
>>   #define ETH_25MHZ_CLK			9
>> +
>> +/*
>> + * The CMN PLL output clock rates that are specifically applicable for IPQ5424
>> + * SoC. For IPQ5424, the other output clocks and their rates are same as IPQ9574.
> 
> Just come with different header. Why was this called in generic way,
> since we ask to name the headers based on compatible?
> 
> Best regards,
> Krzysztof
> 

The DT binding file is named according to the compatible string, however
I had incorrectly assumed that this recommendation was applicable only
to the bindings file. OK, I will add a compatible specific header file
to support the SoC IPQ5424. Thanks.

Re: [PATCH 1/4] dt-bindings: clock: qcom: Add CMN PLL support for IPQ5424 SoC

[Jie Luo]

On 3/25/2025 4:22 PM, Krzysztof Kozlowski wrote:
>>>> --- a/include/dt-bindings/clock/qcom,ipq-cmn-pll.h
>>>> +++ b/include/dt-bindings/clock/qcom,ipq-cmn-pll.h
>>>> @@ -1,6 +1,6 @@
>>>>   /* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
>>>>   /*
>>>> - * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
>>>> + * Copyright (c) 2024-2025 Qualcomm Innovation Center, Inc. All rights reserved.
>>>>    */
>>>>   
>>>>   #ifndef _DT_BINDINGS_CLK_QCOM_IPQ_CMN_PLL_H
>>>> @@ -19,4 +19,12 @@
>>>>   #define ETH1_50MHZ_CLK			7
>>>>   #define ETH2_50MHZ_CLK			8
>>>>   #define ETH_25MHZ_CLK			9
>>>> +
>>>> +/*
>>>> + * The CMN PLL output clock rates that are specifically applicable for IPQ5424
>>>> + * SoC. For IPQ5424, the other output clocks and their rates are same as IPQ9574.
>>>> + */
>>>> +#define NSS_300MHZ_CLK			4
>>>> +#define PPE_375MHZ_CLK			5
>>> Not a huge fan of this, such differences are only relevant to the driver
>>> part in my view - bindings only let a consumer reference a specific piece
>>> of hardware
>> Oh I the bindings are stepping into the frequency department already,
>> hmm.. Then I suppose it's fine if the dt-bindings maintainers don't have any
>> concerns
> 
> Nooooo, it was said these are output clocks, not rates. If these are
> rates, then NAK.
> 
> Best regards,
> Krzysztof

Yes, rates themselves are not defined here, they are defined in the
driver data structure.

The output clocks of CMN PLL are always fixed clock rate, so the clock
frequency was added into the clock specifier macro names defined in
this header file for clarity.

Re: [PATCH 3/4] arm64: dts: ipq5424: Add CMN PLL node

[Konrad Dybcio]

On 3/21/25 1:49 PM, Luo Jie wrote:
> Add CMN PLL node for enabling output clocks to the networking
> hardware blocks on IPQ5424 devices.
> 
> The reference clock of CMN PLL is routed from XO to the CMN PLL
> through the internal WiFi block.
> .XO (48 MHZ or 96 MHZ or 192 MHZ)-->WiFi (multiplier/divider)-->
> 48 MHZ to CMN PLL.
> 
> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
> ---

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

Konrad

Re: [PATCH 4/4] arm64: dts: qcom: Update IPQ5424 xo_board to use fixed factor clock

[Konrad Dybcio]

On 3/21/25 1:49 PM, Luo Jie wrote:
> xo_board is fixed to 24 MHZ, which is routed from WiFi output clock
> 48 MHZ (also being the reference clock of CMN PLL) divided 2 by
> analog block routing channel.
> 
> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
> ---

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

Konrad