2023-02-08 04:29:20

by Kathiravan Thirumoorthy

[permalink] [raw]
Subject: [PATCH V2 0/6] Add APSS clock driver support for IPQ5332

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/[email protected]/

DTS patch depends on the IPQ5332 baseport series
https://lore.kernel.org/linux-arm-msm/[email protected]/

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/[email protected]/

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



2023-02-08 04:29:26

by Kathiravan Thirumoorthy

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

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 <[email protected]>
---
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


2023-02-08 04:29:33

by Kathiravan Thirumoorthy

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

Add IPQ5332 compatible to A53 PLL bindings.

Acked-by: Krzysztof Kozlowski <[email protected]>
Signed-off-by: Kathiravan T <[email protected]>
---
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


2023-02-08 04:29:39

by Kathiravan Thirumoorthy

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

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 <[email protected]>
---
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


2023-02-08 04:30:17

by Kathiravan Thirumoorthy

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

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 <[email protected]>
---
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


2023-02-08 04:30:22

by Kathiravan Thirumoorthy

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

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

Signed-off-by: Kathiravan T <[email protected]>
---
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


2023-02-08 08:02:41

by Krzysztof Kozlowski

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

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 <[email protected]>


Reviewed-by: Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof


2023-02-08 08:41:52

by Konrad Dybcio

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



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 <[email protected]>
> ---
> 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>;

2023-02-08 08:43:32

by Konrad Dybcio

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



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 <[email protected]>
> ---
Reviewed-by: Konrad Dybcio <[email protected]>

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);

2023-02-08 08:45:46

by Konrad Dybcio

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



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 <[email protected]>
> ---
Reviewed-by: Konrad Dybcio <[email protected]>

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 },
> { }

2023-02-09 05:20:52

by Kathiravan Thirumoorthy

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


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 <[email protected]>
>> ---
>> 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>;