[PATCH V2 0/6] Add APSS clock driver support for IPQ5332

[PATCH V2 0/6] Add APSS clock driver support for IPQ5332

[Kathiravan T]

This series adds support for the APSS clock to bump the CPU frequency
above 800MHz.

APSS PLL found in the IPQ5332 is of type Stromer Plus. However the
existing IPQ targets uses the Huayra PLL. So the driver has to
refactored to accommodate the different PLL types. The first patch in
the series does the refactoring, which can be independenty merged.

For the Stromer PLL separate function clk_stromer_pll_configure is
introduced, so the 3rd patch in the series depends on the below patch
https://lore.kernel.org/linux-arm-msm/20230120082631.22053-1-quic_kathirav@quicinc.com/

DTS patch depends on the IPQ5332 baseport series
https://lore.kernel.org/linux-arm-msm/20230130114702.20606-1-quic_kathirav@quicinc.com/

Changes since V1:
	- Dropped the patch 5/6, since the fallback mechanism for compatible
	  is introduced to avoid bloating the of_device_id table
	- V1 can be found at
	  https://lore.kernel.org/linux-arm-msm/20230202145208.2328032-1-quic_kathirav@quicinc.com/

Kathiravan T (5):
  clk: qcom: apss-ipq-pll: refactor the driver to accommodate different
    PLL types
  dt-bindings: clock: qcom,a53pll: add IPQ5332 compatible
  clk: qcom: apss-ipq-pll: add support for IPQ5332
  dt-bindings: mailbox: qcom: add compatible for the IPQ5332 SoC
  arm64: dts: qcom: ipq5332: enable the CPUFreq support

 .../bindings/clock/qcom,a53pll.yaml           |   1 +
 .../mailbox/qcom,apcs-kpss-global.yaml        |  18 ++-
 arch/arm64/boot/dts/qcom/ipq5332.dtsi         |  37 ++++++
 drivers/clk/qcom/apss-ipq-pll.c               | 116 +++++++++++++++---
 4 files changed, 147 insertions(+), 25 deletions(-)

-- 
2.17.1

[PATCH V2 1/5] clk: qcom: apss-ipq-pll: refactor the driver to accommodate different PLL types

[Kathiravan T]

APSS PLL found on the IPQ8074 and IPQ6018 are of type Huayra PLL. But,
IPQ5332 APSS PLL is of type Stromer Plus. To accommodate both these PLLs,
refactor the driver to take the clk_alpha_pll, alpha_pll_config via driver
data.

Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
---
Changes in V2:
	- Added a comment to describe why different offsets are required
	  for PLL

 drivers/clk/qcom/apss-ipq-pll.c | 60 ++++++++++++++++++++++-----------
 1 file changed, 41 insertions(+), 19 deletions(-)

diff --git a/drivers/clk/qcom/apss-ipq-pll.c b/drivers/clk/qcom/apss-ipq-pll.c
index a5aea27eb867..4f2a147e9fb2 100644
--- a/drivers/clk/qcom/apss-ipq-pll.c
+++ b/drivers/clk/qcom/apss-ipq-pll.c
@@ -8,20 +8,27 @@
 
 #include "clk-alpha-pll.h"
 
-static const u8 ipq_pll_offsets[] = {
-	[PLL_OFF_L_VAL] = 0x08,
-	[PLL_OFF_ALPHA_VAL] = 0x10,
-	[PLL_OFF_USER_CTL] = 0x18,
-	[PLL_OFF_CONFIG_CTL] = 0x20,
-	[PLL_OFF_CONFIG_CTL_U] = 0x24,
-	[PLL_OFF_STATUS] = 0x28,
-	[PLL_OFF_TEST_CTL] = 0x30,
-	[PLL_OFF_TEST_CTL_U] = 0x34,
+/*
+ * Even though APSS PLL type is of existing one (like Huayra), its offsets
+ * are different from the one mentioned in the clk-alpha-pll.c, since the
+ * PLL is specific to APSS, so lets the define the same.
+ */
+static const u8 ipq_pll_offsets[][PLL_OFF_MAX_REGS] = {
+	[CLK_ALPHA_PLL_TYPE_HUAYRA] =  {
+		[PLL_OFF_L_VAL] = 0x08,
+		[PLL_OFF_ALPHA_VAL] = 0x10,
+		[PLL_OFF_USER_CTL] = 0x18,
+		[PLL_OFF_CONFIG_CTL] = 0x20,
+		[PLL_OFF_CONFIG_CTL_U] = 0x24,
+		[PLL_OFF_STATUS] = 0x28,
+		[PLL_OFF_TEST_CTL] = 0x30,
+		[PLL_OFF_TEST_CTL_U] = 0x34,
+	},
 };
 
-static struct clk_alpha_pll ipq_pll = {
+static struct clk_alpha_pll ipq_pll_huayra = {
 	.offset = 0x0,
-	.regs = ipq_pll_offsets,
+	.regs = ipq_pll_offsets[CLK_ALPHA_PLL_TYPE_HUAYRA],
 	.flags = SUPPORTS_DYNAMIC_UPDATE,
 	.clkr = {
 		.enable_reg = 0x0,
@@ -61,6 +68,21 @@ static const struct alpha_pll_config ipq8074_pll_config = {
 	.test_ctl_hi_val = 0x4000,
 };
 
+struct apss_pll_data {
+	struct clk_alpha_pll *pll;
+	const struct alpha_pll_config *pll_config;
+};
+
+static struct apss_pll_data ipq8074_pll_data = {
+	.pll = &ipq_pll_huayra,
+	.pll_config = &ipq8074_pll_config,
+};
+
+static struct apss_pll_data ipq6018_pll_data = {
+	.pll = &ipq_pll_huayra,
+	.pll_config = &ipq6018_pll_config,
+};
+
 static const struct regmap_config ipq_pll_regmap_config = {
 	.reg_bits		= 32,
 	.reg_stride		= 4,
@@ -71,7 +93,7 @@ static const struct regmap_config ipq_pll_regmap_config = {
 
 static int apss_ipq_pll_probe(struct platform_device *pdev)
 {
-	const struct alpha_pll_config *ipq_pll_config;
+	const struct apss_pll_data *data;
 	struct device *dev = &pdev->dev;
 	struct regmap *regmap;
 	void __iomem *base;
@@ -85,23 +107,23 @@ static int apss_ipq_pll_probe(struct platform_device *pdev)
 	if (IS_ERR(regmap))
 		return PTR_ERR(regmap);
 
-	ipq_pll_config = of_device_get_match_data(&pdev->dev);
-	if (!ipq_pll_config)
+	data = of_device_get_match_data(&pdev->dev);
+	if (!data)
 		return -ENODEV;
 
-	clk_alpha_pll_configure(&ipq_pll, regmap, ipq_pll_config);
+	clk_alpha_pll_configure(data->pll, regmap, data->pll_config);
 
-	ret = devm_clk_register_regmap(dev, &ipq_pll.clkr);
+	ret = devm_clk_register_regmap(dev, &data->pll->clkr);
 	if (ret)
 		return ret;
 
 	return devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get,
-					   &ipq_pll.clkr.hw);
+					   &data->pll->clkr.hw);
 }
 
 static const struct of_device_id apss_ipq_pll_match_table[] = {
-	{ .compatible = "qcom,ipq6018-a53pll", .data = &ipq6018_pll_config },
-	{ .compatible = "qcom,ipq8074-a53pll", .data = &ipq8074_pll_config },
+	{ .compatible = "qcom,ipq6018-a53pll", .data = &ipq6018_pll_data },
+	{ .compatible = "qcom,ipq8074-a53pll", .data = &ipq8074_pll_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, apss_ipq_pll_match_table);
-- 
2.17.1

[PATCH V2 2/5] dt-bindings: clock: qcom,a53pll: add IPQ5332 compatible

[Kathiravan T]

Add IPQ5332 compatible to A53 PLL bindings.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
---
Changes in V2:
	- Pick up the tags

 Documentation/devicetree/bindings/clock/qcom,a53pll.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml b/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
index 525ebaa93c85..3b6169f30154 100644
--- a/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
@@ -16,6 +16,7 @@ description:
 properties:
   compatible:
     enum:
+      - qcom,ipq5332-a53pll
       - qcom,ipq6018-a53pll
       - qcom,ipq8074-a53pll
       - qcom,msm8916-a53pll
-- 
2.17.1

[PATCH V2 3/5] clk: qcom: apss-ipq-pll: add support for IPQ5332

[Kathiravan T]

IPQ5332 APSS PLL is of type Stromer Plus. Add support for the same.

To configure the stromer plus PLL separate API
(clock_stromer_pll_configure) to be used. To achieve this, introduce the
new member pll_type in device data structure and call the appropriate
function based on this.

Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
---
Changes in V2:
	- No changes

 drivers/clk/qcom/apss-ipq-pll.c | 58 ++++++++++++++++++++++++++++++++-
 1 file changed, 57 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/apss-ipq-pll.c b/drivers/clk/qcom/apss-ipq-pll.c
index 4f2a147e9fb2..cf4f0d340cbf 100644
--- a/drivers/clk/qcom/apss-ipq-pll.c
+++ b/drivers/clk/qcom/apss-ipq-pll.c
@@ -24,6 +24,17 @@ static const u8 ipq_pll_offsets[][PLL_OFF_MAX_REGS] = {
 		[PLL_OFF_TEST_CTL] = 0x30,
 		[PLL_OFF_TEST_CTL_U] = 0x34,
 	},
+	[CLK_ALPHA_PLL_TYPE_STROMER_PLUS] = {
+		[PLL_OFF_L_VAL] = 0x08,
+		[PLL_OFF_ALPHA_VAL] = 0x10,
+		[PLL_OFF_ALPHA_VAL_U] = 0x14,
+		[PLL_OFF_USER_CTL] = 0x18,
+		[PLL_OFF_USER_CTL_U] = 0x1c,
+		[PLL_OFF_CONFIG_CTL] = 0x20,
+		[PLL_OFF_STATUS] = 0x28,
+		[PLL_OFF_TEST_CTL] = 0x30,
+		[PLL_OFF_TEST_CTL_U] = 0x34,
+	},
 };
 
 static struct clk_alpha_pll ipq_pll_huayra = {
@@ -44,6 +55,38 @@ static struct clk_alpha_pll ipq_pll_huayra = {
 	},
 };
 
+static struct clk_alpha_pll ipq_pll_stromer_plus = {
+	.offset = 0x0,
+	.regs = ipq_pll_offsets[CLK_ALPHA_PLL_TYPE_STROMER_PLUS],
+	.flags = SUPPORTS_DYNAMIC_UPDATE,
+	.clkr = {
+		.enable_reg = 0x0,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "a53pll",
+			.parent_data = &(const struct clk_parent_data) {
+				.fw_name = "xo",
+			},
+			.num_parents = 1,
+			.ops = &clk_alpha_pll_stromer_ops,
+		},
+	},
+};
+
+static const struct alpha_pll_config ipq5332_pll_config = {
+	.l = 0x3e,
+	.config_ctl_val = 0x4001075b,
+	.config_ctl_hi_val = 0x304,
+	.main_output_mask = BIT(0),
+	.aux_output_mask = BIT(1),
+	.early_output_mask = BIT(3),
+	.alpha_en_mask = BIT(24),
+	.status_val = 0x3,
+	.status_mask = GENMASK(10, 8),
+	.lock_det = BIT(2),
+	.test_ctl_hi_val = 0x00400003,
+};
+
 static const struct alpha_pll_config ipq6018_pll_config = {
 	.l = 0x37,
 	.config_ctl_val = 0x240d4828,
@@ -69,16 +112,25 @@ static const struct alpha_pll_config ipq8074_pll_config = {
 };
 
 struct apss_pll_data {
+	int pll_type;
 	struct clk_alpha_pll *pll;
 	const struct alpha_pll_config *pll_config;
 };
 
+static struct apss_pll_data ipq5332_pll_data = {
+	.pll_type = CLK_ALPHA_PLL_TYPE_STROMER_PLUS,
+	.pll = &ipq_pll_stromer_plus,
+	.pll_config = &ipq5332_pll_config,
+};
+
 static struct apss_pll_data ipq8074_pll_data = {
+	.pll_type = CLK_ALPHA_PLL_TYPE_HUAYRA,
 	.pll = &ipq_pll_huayra,
 	.pll_config = &ipq8074_pll_config,
 };
 
 static struct apss_pll_data ipq6018_pll_data = {
+	.pll_type = CLK_ALPHA_PLL_TYPE_HUAYRA,
 	.pll = &ipq_pll_huayra,
 	.pll_config = &ipq6018_pll_config,
 };
@@ -111,7 +163,10 @@ static int apss_ipq_pll_probe(struct platform_device *pdev)
 	if (!data)
 		return -ENODEV;
 
-	clk_alpha_pll_configure(data->pll, regmap, data->pll_config);
+	if (data->pll_type == CLK_ALPHA_PLL_TYPE_HUAYRA)
+		clk_alpha_pll_configure(data->pll, regmap, data->pll_config);
+	else if (data->pll_type == CLK_ALPHA_PLL_TYPE_STROMER_PLUS)
+		clk_stromer_pll_configure(data->pll, regmap, data->pll_config);
 
 	ret = devm_clk_register_regmap(dev, &data->pll->clkr);
 	if (ret)
@@ -122,6 +177,7 @@ static int apss_ipq_pll_probe(struct platform_device *pdev)
 }
 
 static const struct of_device_id apss_ipq_pll_match_table[] = {
+	{ .compatible = "qcom,ipq5332-a53pll", .data = &ipq5332_pll_data },
 	{ .compatible = "qcom,ipq6018-a53pll", .data = &ipq6018_pll_data },
 	{ .compatible = "qcom,ipq8074-a53pll", .data = &ipq8074_pll_data },
 	{ }
-- 
2.17.1

[PATCH V2 4/5] dt-bindings: mailbox: qcom: add compatible for the IPQ5332 SoC

[Kathiravan T]

Add the mailbox compatible for the IPQ5332 SoC.

Since the IPQ5332 mailbox is compatible with the IPQ6018, lets create the
fallback to ipq6018 compatible, so that we don't bloat the of_device_id
table in the driver.

Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
---
Changes in V2:
	- As suggested by Krzysztof, modified the binding to use the
	  fallback mechanism so that we don't keep adding the
	  compatibles with the same driver data in mailbox driver

 .../mailbox/qcom,apcs-kpss-global.yaml         | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
index 943f9472ae10..a65b61e5acb0 100644
--- a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
+++ b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
@@ -16,6 +16,10 @@ maintainers:
 properties:
   compatible:
     oneOf:
+      - items:
+          - enum:
+              - qcom,ipq5332-apcs-apps-global
+          - const: qcom,ipq6018-apcs-apps-global
       - items:
           - enum:
               - qcom,ipq6018-apcs-apps-global
@@ -110,9 +114,10 @@ allOf:
   - if:
       properties:
         compatible:
-          enum:
-            - qcom,ipq6018-apcs-apps-global
-            - qcom,ipq8074-apcs-apps-global
+          contains:
+            enum:
+              - qcom,ipq6018-apcs-apps-global
+              - qcom,ipq8074-apcs-apps-global
     then:
       properties:
         clocks:
@@ -126,9 +131,10 @@ allOf:
   - if:
       properties:
         compatible:
-          enum:
-            - qcom,ipq6018-apcs-apps-global
-            - qcom,ipq8074-apcs-apps-global
+          contains:
+            enum:
+              - qcom,ipq6018-apcs-apps-global
+              - qcom,ipq8074-apcs-apps-global
     then:
       properties:
         '#clock-cells':
-- 
2.17.1

[PATCH V2 5/5] arm64: dts: qcom: ipq5332: enable the CPUFreq support

[Kathiravan T]

Add the APCS, A53 PLL, cpu-opp-table nodes to bump the CPU frequency
above 800MHz.

Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
---
Changes in V2:
	- No changes

 arch/arm64/boot/dts/qcom/ipq5332.dtsi | 37 +++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
index bdf33ef30e10..cec2828c51f8 100644
--- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
@@ -5,6 +5,7 @@
  * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved.
  */
 
+#include <dt-bindings/clock/qcom,apss-ipq.h>
 #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 
@@ -35,6 +36,8 @@
 			reg = <0x0>;
 			enable-method = "psci";
 			next-level-cache = <&L2_0>;
+			clocks = <&apcs_glb APCS_ALIAS0_CORE_CLK>;
+			operating-points-v2 = <&cpu_opp_table>;
 		};
 
 		CPU1: cpu@1 {
@@ -43,6 +46,8 @@
 			reg = <0x1>;
 			enable-method = "psci";
 			next-level-cache = <&L2_0>;
+			clocks = <&apcs_glb APCS_ALIAS0_CORE_CLK>;
+			operating-points-v2 = <&cpu_opp_table>;
 		};
 
 		CPU2: cpu@2 {
@@ -51,6 +56,8 @@
 			reg = <0x2>;
 			enable-method = "psci";
 			next-level-cache = <&L2_0>;
+			clocks = <&apcs_glb APCS_ALIAS0_CORE_CLK>;
+			operating-points-v2 = <&cpu_opp_table>;
 		};
 
 		CPU3: cpu@3 {
@@ -59,6 +66,8 @@
 			reg = <0x3>;
 			enable-method = "psci";
 			next-level-cache = <&L2_0>;
+			clocks = <&apcs_glb APCS_ALIAS0_CORE_CLK>;
+			operating-points-v2 = <&cpu_opp_table>;
 		};
 
 		L2_0: l2-cache {
@@ -67,6 +76,16 @@
 		};
 	};
 
+	cpu_opp_table: opp-table-cpu{
+		compatible = "operating-points-v2";
+		opp-shared;
+
+		opp-1488000000 {
+			opp-hz = /bits/ 64 <1488000000>;
+			clock-latency-ns = <200000>;
+		};
+	};
+
 	firmware {
 		scm {
 			compatible = "qcom,scm-ipq5332", "qcom,scm";
@@ -199,6 +218,24 @@
 			};
 		};
 
+		apcs_glb: mailbox@b111000 {
+			compatible = "qcom,ipq5332-apcs-apps-global",
+				     "qcom,ipq6018-apcs-apps-global";
+			reg = <0x0b111000 0x1000>;
+			#clock-cells = <1>;
+			clocks = <&a53pll>, <&xo_board>;
+			clock-names = "pll", "xo";
+			#mbox-cells = <1>;
+		};
+
+		a53pll: clock@b116000 {
+			compatible = "qcom,ipq5332-a53pll";
+			reg = <0x0b116000 0x40>;
+			#clock-cells = <0>;
+			clocks = <&xo_board>;
+			clock-names = "xo";
+		};
+
 		timer@b120000 {
 			compatible = "arm,armv7-timer-mem";
 			reg = <0x0b120000 0x1000>;
-- 
2.17.1

Re: [PATCH V2 4/5] dt-bindings: mailbox: qcom: add compatible for the IPQ5332 SoC

[Krzysztof Kozlowski]

On 08/02/2023 05:28, Kathiravan T wrote:
> Add the mailbox compatible for the IPQ5332 SoC.
> 
> Since the IPQ5332 mailbox is compatible with the IPQ6018, lets create the
> fallback to ipq6018 compatible, so that we don't bloat the of_device_id
> table in the driver.
> 
> Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH V2 5/5] arm64: dts: qcom: ipq5332: enable the CPUFreq support

[Konrad Dybcio]

On 8.02.2023 05:28, Kathiravan T wrote:
> Add the APCS, A53 PLL, cpu-opp-table nodes to bump the CPU frequency
> above 800MHz.
> 
> Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
> ---
> Changes in V2:
> 	- No changes
> 
>  arch/arm64/boot/dts/qcom/ipq5332.dtsi | 37 +++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> index bdf33ef30e10..cec2828c51f8 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> @@ -5,6 +5,7 @@
>   * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved.
>   */
>  
> +#include <dt-bindings/clock/qcom,apss-ipq.h>
>  #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>  
> @@ -35,6 +36,8 @@
>  			reg = <0x0>;
>  			enable-method = "psci";
>  			next-level-cache = <&L2_0>;
> +			clocks = <&apcs_glb APCS_ALIAS0_CORE_CLK>;
> +			operating-points-v2 = <&cpu_opp_table>;
>  		};
>  
>  		CPU1: cpu@1 {
> @@ -43,6 +46,8 @@
>  			reg = <0x1>;
>  			enable-method = "psci";
>  			next-level-cache = <&L2_0>;
> +			clocks = <&apcs_glb APCS_ALIAS0_CORE_CLK>;
> +			operating-points-v2 = <&cpu_opp_table>;
>  		};
>  
>  		CPU2: cpu@2 {
> @@ -51,6 +56,8 @@
>  			reg = <0x2>;
>  			enable-method = "psci";
>  			next-level-cache = <&L2_0>;
> +			clocks = <&apcs_glb APCS_ALIAS0_CORE_CLK>;
> +			operating-points-v2 = <&cpu_opp_table>;
>  		};
>  
>  		CPU3: cpu@3 {
> @@ -59,6 +66,8 @@
>  			reg = <0x3>;
>  			enable-method = "psci";
>  			next-level-cache = <&L2_0>;
> +			clocks = <&apcs_glb APCS_ALIAS0_CORE_CLK>;
> +			operating-points-v2 = <&cpu_opp_table>;
>  		};
>  
>  		L2_0: l2-cache {
> @@ -67,6 +76,16 @@
>  		};
>  	};
>  
> +	cpu_opp_table: opp-table-cpu{
opp-table-cpu {
+ sort this properly (by node name, not label), please

> +		compatible = "operating-points-v2";
> +		opp-shared;
> +
> +		opp-1488000000 {
Why only one (presumably FMAX) target? This sounds
very destructive to power consumption, and by extension
heat output.

The other changes generally look good fwiw.

Konrad
> +			opp-hz = /bits/ 64 <1488000000>;
> +			clock-latency-ns = <200000>;
> +		};
> +	};
> +
>  	firmware {
>  		scm {
>  			compatible = "qcom,scm-ipq5332", "qcom,scm";
> @@ -199,6 +218,24 @@
>  			};
>  		};
>  
> +		apcs_glb: mailbox@b111000 {
> +			compatible = "qcom,ipq5332-apcs-apps-global",
> +				     "qcom,ipq6018-apcs-apps-global";
> +			reg = <0x0b111000 0x1000>;
> +			#clock-cells = <1>;
> +			clocks = <&a53pll>, <&xo_board>;
> +			clock-names = "pll", "xo";
> +			#mbox-cells = <1>;
> +		};
> +
> +		a53pll: clock@b116000 {
> +			compatible = "qcom,ipq5332-a53pll";
> +			reg = <0x0b116000 0x40>;
> +			#clock-cells = <0>;
> +			clocks = <&xo_board>;
> +			clock-names = "xo";
> +		};
> +
>  		timer@b120000 {
>  			compatible = "arm,armv7-timer-mem";
>  			reg = <0x0b120000 0x1000>;

Re: [PATCH V2 1/5] clk: qcom: apss-ipq-pll: refactor the driver to accommodate different PLL types

[Konrad Dybcio]

On 8.02.2023 05:28, Kathiravan T wrote:
> APSS PLL found on the IPQ8074 and IPQ6018 are of type Huayra PLL. But,
> IPQ5332 APSS PLL is of type Stromer Plus. To accommodate both these PLLs,
> refactor the driver to take the clk_alpha_pll, alpha_pll_config via driver
> data.
> 
> Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
> Changes in V2:
> 	- Added a comment to describe why different offsets are required
> 	  for PLL
> 
>  drivers/clk/qcom/apss-ipq-pll.c | 60 ++++++++++++++++++++++-----------
>  1 file changed, 41 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/clk/qcom/apss-ipq-pll.c b/drivers/clk/qcom/apss-ipq-pll.c
> index a5aea27eb867..4f2a147e9fb2 100644
> --- a/drivers/clk/qcom/apss-ipq-pll.c
> +++ b/drivers/clk/qcom/apss-ipq-pll.c
> @@ -8,20 +8,27 @@
>  
>  #include "clk-alpha-pll.h"
>  
> -static const u8 ipq_pll_offsets[] = {
> -	[PLL_OFF_L_VAL] = 0x08,
> -	[PLL_OFF_ALPHA_VAL] = 0x10,
> -	[PLL_OFF_USER_CTL] = 0x18,
> -	[PLL_OFF_CONFIG_CTL] = 0x20,
> -	[PLL_OFF_CONFIG_CTL_U] = 0x24,
> -	[PLL_OFF_STATUS] = 0x28,
> -	[PLL_OFF_TEST_CTL] = 0x30,
> -	[PLL_OFF_TEST_CTL_U] = 0x34,
> +/*
> + * Even though APSS PLL type is of existing one (like Huayra), its offsets
> + * are different from the one mentioned in the clk-alpha-pll.c, since the
> + * PLL is specific to APSS, so lets the define the same.
> + */
> +static const u8 ipq_pll_offsets[][PLL_OFF_MAX_REGS] = {
> +	[CLK_ALPHA_PLL_TYPE_HUAYRA] =  {
> +		[PLL_OFF_L_VAL] = 0x08,
> +		[PLL_OFF_ALPHA_VAL] = 0x10,
> +		[PLL_OFF_USER_CTL] = 0x18,
> +		[PLL_OFF_CONFIG_CTL] = 0x20,
> +		[PLL_OFF_CONFIG_CTL_U] = 0x24,
> +		[PLL_OFF_STATUS] = 0x28,
> +		[PLL_OFF_TEST_CTL] = 0x30,
> +		[PLL_OFF_TEST_CTL_U] = 0x34,
> +	},
>  };
>  
> -static struct clk_alpha_pll ipq_pll = {
> +static struct clk_alpha_pll ipq_pll_huayra = {
>  	.offset = 0x0,
> -	.regs = ipq_pll_offsets,
> +	.regs = ipq_pll_offsets[CLK_ALPHA_PLL_TYPE_HUAYRA],
>  	.flags = SUPPORTS_DYNAMIC_UPDATE,
>  	.clkr = {
>  		.enable_reg = 0x0,
> @@ -61,6 +68,21 @@ static const struct alpha_pll_config ipq8074_pll_config = {
>  	.test_ctl_hi_val = 0x4000,
>  };
>  
> +struct apss_pll_data {
> +	struct clk_alpha_pll *pll;
> +	const struct alpha_pll_config *pll_config;
> +};
> +
> +static struct apss_pll_data ipq8074_pll_data = {
> +	.pll = &ipq_pll_huayra,
> +	.pll_config = &ipq8074_pll_config,
> +};
> +
> +static struct apss_pll_data ipq6018_pll_data = {
> +	.pll = &ipq_pll_huayra,
> +	.pll_config = &ipq6018_pll_config,
> +};
> +
>  static const struct regmap_config ipq_pll_regmap_config = {
>  	.reg_bits		= 32,
>  	.reg_stride		= 4,
> @@ -71,7 +93,7 @@ static const struct regmap_config ipq_pll_regmap_config = {
>  
>  static int apss_ipq_pll_probe(struct platform_device *pdev)
>  {
> -	const struct alpha_pll_config *ipq_pll_config;
> +	const struct apss_pll_data *data;
>  	struct device *dev = &pdev->dev;
>  	struct regmap *regmap;
>  	void __iomem *base;
> @@ -85,23 +107,23 @@ static int apss_ipq_pll_probe(struct platform_device *pdev)
>  	if (IS_ERR(regmap))
>  		return PTR_ERR(regmap);
>  
> -	ipq_pll_config = of_device_get_match_data(&pdev->dev);
> -	if (!ipq_pll_config)
> +	data = of_device_get_match_data(&pdev->dev);
> +	if (!data)
>  		return -ENODEV;
>  
> -	clk_alpha_pll_configure(&ipq_pll, regmap, ipq_pll_config);
> +	clk_alpha_pll_configure(data->pll, regmap, data->pll_config);
>  
> -	ret = devm_clk_register_regmap(dev, &ipq_pll.clkr);
> +	ret = devm_clk_register_regmap(dev, &data->pll->clkr);
>  	if (ret)
>  		return ret;
>  
>  	return devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get,
> -					   &ipq_pll.clkr.hw);
> +					   &data->pll->clkr.hw);
>  }
>  
>  static const struct of_device_id apss_ipq_pll_match_table[] = {
> -	{ .compatible = "qcom,ipq6018-a53pll", .data = &ipq6018_pll_config },
> -	{ .compatible = "qcom,ipq8074-a53pll", .data = &ipq8074_pll_config },
> +	{ .compatible = "qcom,ipq6018-a53pll", .data = &ipq6018_pll_data },
> +	{ .compatible = "qcom,ipq8074-a53pll", .data = &ipq8074_pll_data },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, apss_ipq_pll_match_table);

Re: [PATCH V2 3/5] clk: qcom: apss-ipq-pll: add support for IPQ5332

[Konrad Dybcio]

On 8.02.2023 05:28, Kathiravan T wrote:
> IPQ5332 APSS PLL is of type Stromer Plus. Add support for the same.
> 
> To configure the stromer plus PLL separate API
> (clock_stromer_pll_configure) to be used. To achieve this, introduce the
> new member pll_type in device data structure and call the appropriate
> function based on this.
> 
> Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
> Changes in V2:
> 	- No changes
> 
>  drivers/clk/qcom/apss-ipq-pll.c | 58 ++++++++++++++++++++++++++++++++-
>  1 file changed, 57 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/qcom/apss-ipq-pll.c b/drivers/clk/qcom/apss-ipq-pll.c
> index 4f2a147e9fb2..cf4f0d340cbf 100644
> --- a/drivers/clk/qcom/apss-ipq-pll.c
> +++ b/drivers/clk/qcom/apss-ipq-pll.c
> @@ -24,6 +24,17 @@ static const u8 ipq_pll_offsets[][PLL_OFF_MAX_REGS] = {
>  		[PLL_OFF_TEST_CTL] = 0x30,
>  		[PLL_OFF_TEST_CTL_U] = 0x34,
>  	},
> +	[CLK_ALPHA_PLL_TYPE_STROMER_PLUS] = {
> +		[PLL_OFF_L_VAL] = 0x08,
> +		[PLL_OFF_ALPHA_VAL] = 0x10,
> +		[PLL_OFF_ALPHA_VAL_U] = 0x14,
> +		[PLL_OFF_USER_CTL] = 0x18,
> +		[PLL_OFF_USER_CTL_U] = 0x1c,
> +		[PLL_OFF_CONFIG_CTL] = 0x20,
> +		[PLL_OFF_STATUS] = 0x28,
> +		[PLL_OFF_TEST_CTL] = 0x30,
> +		[PLL_OFF_TEST_CTL_U] = 0x34,
> +	},
>  };
>  
>  static struct clk_alpha_pll ipq_pll_huayra = {
> @@ -44,6 +55,38 @@ static struct clk_alpha_pll ipq_pll_huayra = {
>  	},
>  };
>  
> +static struct clk_alpha_pll ipq_pll_stromer_plus = {
> +	.offset = 0x0,
> +	.regs = ipq_pll_offsets[CLK_ALPHA_PLL_TYPE_STROMER_PLUS],
> +	.flags = SUPPORTS_DYNAMIC_UPDATE,
> +	.clkr = {
> +		.enable_reg = 0x0,
> +		.enable_mask = BIT(0),
> +		.hw.init = &(struct clk_init_data){
> +			.name = "a53pll",
> +			.parent_data = &(const struct clk_parent_data) {
> +				.fw_name = "xo",
> +			},
> +			.num_parents = 1,
> +			.ops = &clk_alpha_pll_stromer_ops,
> +		},
> +	},
> +};
> +
> +static const struct alpha_pll_config ipq5332_pll_config = {
> +	.l = 0x3e,
> +	.config_ctl_val = 0x4001075b,
> +	.config_ctl_hi_val = 0x304,
> +	.main_output_mask = BIT(0),
> +	.aux_output_mask = BIT(1),
> +	.early_output_mask = BIT(3),
> +	.alpha_en_mask = BIT(24),
> +	.status_val = 0x3,
> +	.status_mask = GENMASK(10, 8),
> +	.lock_det = BIT(2),
> +	.test_ctl_hi_val = 0x00400003,
> +};
> +
>  static const struct alpha_pll_config ipq6018_pll_config = {
>  	.l = 0x37,
>  	.config_ctl_val = 0x240d4828,
> @@ -69,16 +112,25 @@ static const struct alpha_pll_config ipq8074_pll_config = {
>  };
>  
>  struct apss_pll_data {
> +	int pll_type;
>  	struct clk_alpha_pll *pll;
>  	const struct alpha_pll_config *pll_config;
>  };
>  
> +static struct apss_pll_data ipq5332_pll_data = {
> +	.pll_type = CLK_ALPHA_PLL_TYPE_STROMER_PLUS,
> +	.pll = &ipq_pll_stromer_plus,
> +	.pll_config = &ipq5332_pll_config,
> +};
> +
>  static struct apss_pll_data ipq8074_pll_data = {
> +	.pll_type = CLK_ALPHA_PLL_TYPE_HUAYRA,
>  	.pll = &ipq_pll_huayra,
>  	.pll_config = &ipq8074_pll_config,
>  };
>  
>  static struct apss_pll_data ipq6018_pll_data = {
> +	.pll_type = CLK_ALPHA_PLL_TYPE_HUAYRA,
>  	.pll = &ipq_pll_huayra,
>  	.pll_config = &ipq6018_pll_config,
>  };
> @@ -111,7 +163,10 @@ static int apss_ipq_pll_probe(struct platform_device *pdev)
>  	if (!data)
>  		return -ENODEV;
>  
> -	clk_alpha_pll_configure(data->pll, regmap, data->pll_config);
> +	if (data->pll_type == CLK_ALPHA_PLL_TYPE_HUAYRA)
> +		clk_alpha_pll_configure(data->pll, regmap, data->pll_config);
> +	else if (data->pll_type == CLK_ALPHA_PLL_TYPE_STROMER_PLUS)
> +		clk_stromer_pll_configure(data->pll, regmap, data->pll_config);
>  
>  	ret = devm_clk_register_regmap(dev, &data->pll->clkr);
>  	if (ret)
> @@ -122,6 +177,7 @@ static int apss_ipq_pll_probe(struct platform_device *pdev)
>  }
>  
>  static const struct of_device_id apss_ipq_pll_match_table[] = {
> +	{ .compatible = "qcom,ipq5332-a53pll", .data = &ipq5332_pll_data },
>  	{ .compatible = "qcom,ipq6018-a53pll", .data = &ipq6018_pll_data },
>  	{ .compatible = "qcom,ipq8074-a53pll", .data = &ipq8074_pll_data },
>  	{ }

Re: [PATCH V2 5/5] arm64: dts: qcom: ipq5332: enable the CPUFreq support

[Kathiravan T]

On 2/8/2023 2:11 PM, Konrad Dybcio wrote:
>
> On 8.02.2023 05:28, Kathiravan T wrote:
>> Add the APCS, A53 PLL, cpu-opp-table nodes to bump the CPU frequency
>> above 800MHz.
>>
>> Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
>> ---
>> Changes in V2:
>> 	- No changes
>>
>>   arch/arm64/boot/dts/qcom/ipq5332.dtsi | 37 +++++++++++++++++++++++++++
>>   1 file changed, 37 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
>> index bdf33ef30e10..cec2828c51f8 100644
>> --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
>> @@ -5,6 +5,7 @@
>>    * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved.
>>    */
>>   
>> +#include <dt-bindings/clock/qcom,apss-ipq.h>
>>   #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
>>   #include <dt-bindings/interrupt-controller/arm-gic.h>
>>   
>> @@ -35,6 +36,8 @@
>>   			reg = <0x0>;
>>   			enable-method = "psci";
>>   			next-level-cache = <&L2_0>;
>> +			clocks = <&apcs_glb APCS_ALIAS0_CORE_CLK>;
>> +			operating-points-v2 = <&cpu_opp_table>;
>>   		};
>>   
>>   		CPU1: cpu@1 {
>> @@ -43,6 +46,8 @@
>>   			reg = <0x1>;
>>   			enable-method = "psci";
>>   			next-level-cache = <&L2_0>;
>> +			clocks = <&apcs_glb APCS_ALIAS0_CORE_CLK>;
>> +			operating-points-v2 = <&cpu_opp_table>;
>>   		};
>>   
>>   		CPU2: cpu@2 {
>> @@ -51,6 +56,8 @@
>>   			reg = <0x2>;
>>   			enable-method = "psci";
>>   			next-level-cache = <&L2_0>;
>> +			clocks = <&apcs_glb APCS_ALIAS0_CORE_CLK>;
>> +			operating-points-v2 = <&cpu_opp_table>;
>>   		};
>>   
>>   		CPU3: cpu@3 {
>> @@ -59,6 +66,8 @@
>>   			reg = <0x3>;
>>   			enable-method = "psci";
>>   			next-level-cache = <&L2_0>;
>> +			clocks = <&apcs_glb APCS_ALIAS0_CORE_CLK>;
>> +			operating-points-v2 = <&cpu_opp_table>;
>>   		};
>>   
>>   		L2_0: l2-cache {
>> @@ -67,6 +76,16 @@
>>   		};
>>   	};
>>   
>> +	cpu_opp_table: opp-table-cpu{
> opp-table-cpu {
> + sort this properly (by node name, not label), please


ahh, missed this. Will fix it in next spin.


>
>> +		compatible = "operating-points-v2";
>> +		opp-shared;
>> +
>> +		opp-1488000000 {
> Why only one (presumably FMAX) target? This sounds
> very destructive to power consumption, and by extension
> heat output.


SKU is designed to operate on 1.48GHz only.


>
> The other changes generally look good fwiw.
>
> Konrad
>> +			opp-hz = /bits/ 64 <1488000000>;
>> +			clock-latency-ns = <200000>;
>> +		};
>> +	};
>> +
>>   	firmware {
>>   		scm {
>>   			compatible = "qcom,scm-ipq5332", "qcom,scm";
>> @@ -199,6 +218,24 @@
>>   			};
>>   		};
>>   
>> +		apcs_glb: mailbox@b111000 {
>> +			compatible = "qcom,ipq5332-apcs-apps-global",
>> +				     "qcom,ipq6018-apcs-apps-global";
>> +			reg = <0x0b111000 0x1000>;
>> +			#clock-cells = <1>;
>> +			clocks = <&a53pll>, <&xo_board>;
>> +			clock-names = "pll", "xo";
>> +			#mbox-cells = <1>;
>> +		};
>> +
>> +		a53pll: clock@b116000 {
>> +			compatible = "qcom,ipq5332-a53pll";
>> +			reg = <0x0b116000 0x40>;
>> +			#clock-cells = <0>;
>> +			clocks = <&xo_board>;
>> +			clock-names = "xo";
>> +		};
>> +
>>   		timer@b120000 {
>>   			compatible = "arm,armv7-timer-mem";
>>   			reg = <0x0b120000 0x1000>;