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]/
Kathiravan T (6):
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
mailbox: qcom-apcs-ipc: add IPQ5332 APSS clock support
arm64: dts: qcom: ipq5332: enable the CPUFreq support
.../bindings/clock/qcom,a53pll.yaml | 1 +
.../mailbox/qcom,apcs-kpss-global.yaml | 3 +
arch/arm64/boot/dts/qcom/ipq5332.dtsi | 36 ++++++
drivers/clk/qcom/apss-ipq-pll.c | 111 +++++++++++++++---
drivers/mailbox/qcom-apcs-ipc-mailbox.c | 1 +
5 files changed, 133 insertions(+), 19 deletions(-)
--
2.34.1
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 device
data.
Signed-off-by: Kathiravan T <[email protected]>
---
drivers/clk/qcom/apss-ipq-pll.c | 55 +++++++++++++++++++++------------
1 file changed, 36 insertions(+), 19 deletions(-)
diff --git a/drivers/clk/qcom/apss-ipq-pll.c b/drivers/clk/qcom/apss-ipq-pll.c
index a5aea27eb867..6e815e8b7fe4 100644
--- a/drivers/clk/qcom/apss-ipq-pll.c
+++ b/drivers/clk/qcom/apss-ipq-pll.c
@@ -8,20 +8,22 @@
#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,
+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 +63,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 +88,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 +102,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.34.1
Add IPQ5332 compatible to A53 PLL bindings.
Signed-off-by: Kathiravan T <[email protected]>
---
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.34.1
Add the mailbox compatible for the IPQ5332 SoC.
Signed-off-by: Kathiravan T <[email protected]>
---
.../devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
index 943f9472ae10..8d8cd1bbe67e 100644
--- a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
+++ b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
@@ -18,6 +18,7 @@ properties:
oneOf:
- items:
- enum:
+ - qcom,ipq5332-apcs-apps-global
- qcom,ipq6018-apcs-apps-global
- qcom,ipq8074-apcs-apps-global
- qcom,msm8976-apcs-kpss-global
@@ -111,6 +112,7 @@ allOf:
properties:
compatible:
enum:
+ - qcom,ipq5332-apcs-apps-global
- qcom,ipq6018-apcs-apps-global
- qcom,ipq8074-apcs-apps-global
then:
@@ -127,6 +129,7 @@ allOf:
properties:
compatible:
enum:
+ - qcom,ipq5332-apcs-apps-global
- qcom,ipq6018-apcs-apps-global
- qcom,ipq8074-apcs-apps-global
then:
--
2.34.1
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]>
---
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 6e815e8b7fe4..023a854f2c21 100644
--- a/drivers/clk/qcom/apss-ipq-pll.c
+++ b/drivers/clk/qcom/apss-ipq-pll.c
@@ -19,6 +19,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 = {
@@ -39,6 +50,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,
@@ -64,16 +107,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,
};
@@ -106,7 +158,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)
@@ -117,6 +172,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.34.1
IPQ5332 has the APSS clock controller utilizing the same register space
as the APCS, so provide access to the APSS utilizing a child device like
other IPQ chipsets.
Like IPQ6018, the same controller and driver is used, so utilize IPQ6018
match data for IPQ5332.
Signed-off-by: Kathiravan T <[email protected]>
---
drivers/mailbox/qcom-apcs-ipc-mailbox.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mailbox/qcom-apcs-ipc-mailbox.c b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
index 0e9f9cba8668..9d1f1b8671fc 100644
--- a/drivers/mailbox/qcom-apcs-ipc-mailbox.c
+++ b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
@@ -141,6 +141,7 @@ static int qcom_apcs_ipc_remove(struct platform_device *pdev)
/* .data is the offset of the ipc register within the global block */
static const struct of_device_id qcom_apcs_ipc_of_match[] = {
+ { .compatible = "qcom,ipq5332-apcs-apps-global", .data = &ipq6018_apcs_data },
{ .compatible = "qcom,ipq6018-apcs-apps-global", .data = &ipq6018_apcs_data },
{ .compatible = "qcom,ipq8074-apcs-apps-global", .data = &ipq6018_apcs_data },
{ .compatible = "qcom,msm8916-apcs-kpss-global", .data = &msm8916_apcs_data },
--
2.34.1
Add the APCS, A53 PLL, cpu-opp-table nodes to enable the CPU frequency
above 800MHz.
Signed-off-by: Kathiravan T <[email protected]>
---
arch/arm64/boot/dts/qcom/ipq5332.dtsi | 36 +++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
index 7f0ba2ec339c..483bd89817b6 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 @@ CPU0: cpu@0 {
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 @@ CPU1: cpu@1 {
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 @@ CPU2: cpu@2 {
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 @@ CPU3: cpu@3 {
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 @@ L2_0: l2-cache {
};
};
+ 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";
@@ -203,6 +222,23 @@ v2m2: v2m@2 {
};
};
+ apcs_glb: mailbox@b111000 {
+ compatible = "qcom,ipq5332-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.34.1
On 2.02.2023 15:52, 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 device
> data.
>
> Signed-off-by: Kathiravan T <[email protected]>
> ---
> drivers/clk/qcom/apss-ipq-pll.c | 55 +++++++++++++++++++++------------
> 1 file changed, 36 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/clk/qcom/apss-ipq-pll.c b/drivers/clk/qcom/apss-ipq-pll.c
> index a5aea27eb867..6e815e8b7fe4 100644
> --- a/drivers/clk/qcom/apss-ipq-pll.c
> +++ b/drivers/clk/qcom/apss-ipq-pll.c
> @@ -8,20 +8,22 @@
>
> #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,
> +static const u8 ipq_pll_offsets[][PLL_OFF_MAX_REGS] = {
> + [CLK_ALPHA_PLL_TYPE_HUAYRA] = {
Is it really huayra? The definition in clk-alpha-pll.c is
different..
Konrad
> + [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 +63,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 +88,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 +102,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);
On 2.02.2023 15:52, Kathiravan T wrote:
> IPQ5332 has the APSS clock controller utilizing the same register space
> as the APCS, so provide access to the APSS utilizing a child device like
> other IPQ chipsets.
>
> Like IPQ6018, the same controller and driver is used, so utilize IPQ6018
> match data for IPQ5332.
>
> Signed-off-by: Kathiravan T <[email protected]>
> ---
Reviewed-by: Konrad Dybcio <[email protected]>
Konrad
> drivers/mailbox/qcom-apcs-ipc-mailbox.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mailbox/qcom-apcs-ipc-mailbox.c b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
> index 0e9f9cba8668..9d1f1b8671fc 100644
> --- a/drivers/mailbox/qcom-apcs-ipc-mailbox.c
> +++ b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
> @@ -141,6 +141,7 @@ static int qcom_apcs_ipc_remove(struct platform_device *pdev)
>
> /* .data is the offset of the ipc register within the global block */
> static const struct of_device_id qcom_apcs_ipc_of_match[] = {
> + { .compatible = "qcom,ipq5332-apcs-apps-global", .data = &ipq6018_apcs_data },
> { .compatible = "qcom,ipq6018-apcs-apps-global", .data = &ipq6018_apcs_data },
> { .compatible = "qcom,ipq8074-apcs-apps-global", .data = &ipq6018_apcs_data },
> { .compatible = "qcom,msm8916-apcs-kpss-global", .data = &msm8916_apcs_data },
On 2.02.2023 15:52, 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]>
> ---
> 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 6e815e8b7fe4..023a854f2c21 100644
> --- a/drivers/clk/qcom/apss-ipq-pll.c
> +++ b/drivers/clk/qcom/apss-ipq-pll.c
> @@ -19,6 +19,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,
> + },
Any reason this couldn't use clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_STROMER_PLUS]
exported from clk-alpha-pll.c?
Konrad
> };
>
> static struct clk_alpha_pll ipq_pll_huayra = {
> @@ -39,6 +50,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,
> @@ -64,16 +107,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,
> };
> @@ -106,7 +158,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)
> @@ -117,6 +172,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 },
> { }
On 02/02/2023 15:52, Kathiravan T wrote:
> Add IPQ5332 compatible to A53 PLL bindings.
>
> Signed-off-by: Kathiravan T <[email protected]>
> ---
Acked-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
On 02/02/2023 16:16, Konrad Dybcio wrote:
>
>
> On 2.02.2023 15:52, Kathiravan T wrote:
>> IPQ5332 has the APSS clock controller utilizing the same register space
>> as the APCS, so provide access to the APSS utilizing a child device like
>> other IPQ chipsets.
>>
>> Like IPQ6018, the same controller and driver is used, so utilize IPQ6018
>> match data for IPQ5332.
>>
>> Signed-off-by: Kathiravan T <[email protected]>
>> ---
> Reviewed-by: Konrad Dybcio <[email protected]>
While this is not the fault of this patch, but we keep adding
compatibles with same driver data. I think this should start using
fallbacks at some point...
Best regards,
Krzysztof
On 02/02/2023 15:52, Kathiravan T wrote:
> Add the mailbox compatible for the IPQ5332 SoC.
>
> Signed-off-by: Kathiravan T <[email protected]>
> ---
> .../devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
> index 943f9472ae10..8d8cd1bbe67e 100644
> --- a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
> +++ b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
> @@ -18,6 +18,7 @@ properties:
> oneOf:
- items:
- enum:
- qcom,ipq5332-apcs-apps-global
- const: qcom,ipq6018-apcs-apps-global
and drop the next patch
Best regards,
Krzysztof
On 2/2/2023 8:48 PM, Konrad Dybcio wrote:
>
> On 2.02.2023 15:52, 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]>
>> ---
>> 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 6e815e8b7fe4..023a854f2c21 100644
>> --- a/drivers/clk/qcom/apss-ipq-pll.c
>> +++ b/drivers/clk/qcom/apss-ipq-pll.c
>> @@ -19,6 +19,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,
>> + },
> Any reason this couldn't use clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_STROMER_PLUS]
> exported from clk-alpha-pll.c?
Same reason as Huayra. The APSS PLL offset is different than the one
exposed in clk-alpha-pll.c
Thanks,
Kathiravan T.
>
> Konrad
>> };
>>
>> static struct clk_alpha_pll ipq_pll_huayra = {
>> @@ -39,6 +50,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,
>> @@ -64,16 +107,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,
>> };
>> @@ -106,7 +158,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)
>> @@ -117,6 +172,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 },
>> { }
On 2/2/2023 9:05 PM, Krzysztof Kozlowski wrote:
> On 02/02/2023 15:52, Kathiravan T wrote:
>> Add the mailbox compatible for the IPQ5332 SoC.
>>
>> Signed-off-by: Kathiravan T <[email protected]>
>> ---
>> .../devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
>> index 943f9472ae10..8d8cd1bbe67e 100644
>> --- a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
>> +++ b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
>> @@ -18,6 +18,7 @@ properties:
>> oneOf:
> - items:
> - enum:
> - qcom,ipq5332-apcs-apps-global
> - const: qcom,ipq6018-apcs-apps-global
>
> and drop the next patch
Thanks, will update in V2, as suggested.
>
>
> Best regards,
> Krzysztof
>
On 02/02/2023 16:30, Krzysztof Kozlowski wrote:
> On 02/02/2023 16:16, Konrad Dybcio wrote:
>>
>>
>> On 2.02.2023 15:52, Kathiravan T wrote:
>>> IPQ5332 has the APSS clock controller utilizing the same register space
>>> as the APCS, so provide access to the APSS utilizing a child device like
>>> other IPQ chipsets.
>>>
>>> Like IPQ6018, the same controller and driver is used, so utilize IPQ6018
>>> match data for IPQ5332.
>>>
>>> Signed-off-by: Kathiravan T <[email protected]>
>>> ---
>> Reviewed-by: Konrad Dybcio <[email protected]>
>
> While this is not the fault of this patch, but we keep adding
> compatibles with same driver data. I think this should start using
> fallbacks at some point...
I'll work on this.
Best regards,
Krzysztof
On 2.02.2023 16:37, Kathiravan T wrote:
>
> On 2/2/2023 8:45 PM, Konrad Dybcio wrote:
>>
>> On 2.02.2023 15:52, 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 device
>>> data.
>>>
>>> Signed-off-by: Kathiravan T <[email protected]>
>>> ---
>>> drivers/clk/qcom/apss-ipq-pll.c | 55 +++++++++++++++++++++------------
>>> 1 file changed, 36 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/drivers/clk/qcom/apss-ipq-pll.c b/drivers/clk/qcom/apss-ipq-pll.c
>>> index a5aea27eb867..6e815e8b7fe4 100644
>>> --- a/drivers/clk/qcom/apss-ipq-pll.c
>>> +++ b/drivers/clk/qcom/apss-ipq-pll.c
>>> @@ -8,20 +8,22 @@
>>> #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,
>>> +static const u8 ipq_pll_offsets[][PLL_OFF_MAX_REGS] = {
>>> + [CLK_ALPHA_PLL_TYPE_HUAYRA] = {
>> Is it really huayra? The definition in clk-alpha-pll.c is
>> different..
>
>
> As per the HW document, yes it is Huayra. When you say "different", I understand these offsets are not matching with the one in clk-alpha-pll.c right? I checked with the sub system owner and they said it is expected since these PLLs are specific to each subsystem(A53 and GCC), so the offsets are different.
Okay, that makes sense. Could you please leave a comment about this
in the code?
Konrad
>
>
> Thanks,
>
> Kathiravan T.
>
>
>>
>>
>> Konrad
>>> + [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 +63,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 +88,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 +102,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);
On 2/2/2023 9:26 PM, Konrad Dybcio wrote:
>
> On 2.02.2023 16:37, Kathiravan T wrote:
>> On 2/2/2023 8:45 PM, Konrad Dybcio wrote:
>>> On 2.02.2023 15:52, 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 device
>>>> data.
>>>>
>>>> Signed-off-by: Kathiravan T <[email protected]>
>>>> ---
>>>> drivers/clk/qcom/apss-ipq-pll.c | 55 +++++++++++++++++++++------------
>>>> 1 file changed, 36 insertions(+), 19 deletions(-)
>>>>
>>>> diff --git a/drivers/clk/qcom/apss-ipq-pll.c b/drivers/clk/qcom/apss-ipq-pll.c
>>>> index a5aea27eb867..6e815e8b7fe4 100644
>>>> --- a/drivers/clk/qcom/apss-ipq-pll.c
>>>> +++ b/drivers/clk/qcom/apss-ipq-pll.c
>>>> @@ -8,20 +8,22 @@
>>>> #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,
>>>> +static const u8 ipq_pll_offsets[][PLL_OFF_MAX_REGS] = {
>>>> + [CLK_ALPHA_PLL_TYPE_HUAYRA] = {
>>> Is it really huayra? The definition in clk-alpha-pll.c is
>>> different..
>>
>> As per the HW document, yes it is Huayra. When you say "different", I understand these offsets are not matching with the one in clk-alpha-pll.c right? I checked with the sub system owner and they said it is expected since these PLLs are specific to each subsystem(A53 and GCC), so the offsets are different.
> Okay, that makes sense. Could you please leave a comment about this
> in the code?
Sure, will do it in V2.
> Konrad
>>
>> Thanks,
>>
>> Kathiravan T.
>>
>>
>>>
>>> Konrad
>>>> + [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 +63,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 +88,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 +102,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);
On 2/2/2023 8:45 PM, Konrad Dybcio wrote:
>
> On 2.02.2023 15:52, 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 device
>> data.
>>
>> Signed-off-by: Kathiravan T <[email protected]>
>> ---
>> drivers/clk/qcom/apss-ipq-pll.c | 55 +++++++++++++++++++++------------
>> 1 file changed, 36 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/apss-ipq-pll.c b/drivers/clk/qcom/apss-ipq-pll.c
>> index a5aea27eb867..6e815e8b7fe4 100644
>> --- a/drivers/clk/qcom/apss-ipq-pll.c
>> +++ b/drivers/clk/qcom/apss-ipq-pll.c
>> @@ -8,20 +8,22 @@
>>
>> #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,
>> +static const u8 ipq_pll_offsets[][PLL_OFF_MAX_REGS] = {
>> + [CLK_ALPHA_PLL_TYPE_HUAYRA] = {
> Is it really huayra? The definition in clk-alpha-pll.c is
> different..
As per the HW document, yes it is Huayra. When you say "different", I
understand these offsets are not matching with the one in
clk-alpha-pll.c right? I checked with the sub system owner and they said
it is expected since these PLLs are specific to each subsystem(A53 and
GCC), so the offsets are different.
Thanks,
Kathiravan T.
>
>
> Konrad
>> + [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 +63,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 +88,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 +102,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);
On 2.02.2023 16:30, Krzysztof Kozlowski wrote:
> On 02/02/2023 16:16, Konrad Dybcio wrote:
>>
>>
>> On 2.02.2023 15:52, Kathiravan T wrote:
>>> IPQ5332 has the APSS clock controller utilizing the same register space
>>> as the APCS, so provide access to the APSS utilizing a child device like
>>> other IPQ chipsets.
>>>
>>> Like IPQ6018, the same controller and driver is used, so utilize IPQ6018
>>> match data for IPQ5332.
>>>
>>> Signed-off-by: Kathiravan T <[email protected]>
>>> ---
>> Reviewed-by: Konrad Dybcio <[email protected]>
>
> While this is not the fault of this patch, but we keep adding
> compatibles with same driver data. I think this should start using
> fallbacks at some point...
Fair.
Konrad
>
> Best regards,
> Krzysztof
>
On 02/02/2023 17:35, Krzysztof Kozlowski wrote:
> On 02/02/2023 15:52, Kathiravan T wrote:
>> Add the mailbox compatible for the IPQ5332 SoC.
>>
>> Signed-off-by: Kathiravan T <[email protected]>
>> ---
>> .../devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
>> index 943f9472ae10..8d8cd1bbe67e 100644
>> --- a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
>> +++ b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
>> @@ -18,6 +18,7 @@ properties:
>> oneOf:
>
> - items:
> - enum:
> - qcom,ipq5332-apcs-apps-global
> - const: qcom,ipq6018-apcs-apps-global
>
> and drop the next patch
Is it still ok even if the two devices are not fully compatible (iow,
using different PLL types)?
--
With best wishes
Dmitry
On 02/02/2023 22:00, Dmitry Baryshkov wrote:
> On 02/02/2023 17:35, Krzysztof Kozlowski wrote:
>> On 02/02/2023 15:52, Kathiravan T wrote:
>>> Add the mailbox compatible for the IPQ5332 SoC.
>>>
>>> Signed-off-by: Kathiravan T <[email protected]>
>>> ---
>>> .../devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
>>> index 943f9472ae10..8d8cd1bbe67e 100644
>>> ---
>>> a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
>>> +++
>>> b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
>>> @@ -18,6 +18,7 @@ properties:
>>> oneOf:
>>
>> - items:
>> - enum:
>> - qcom,ipq5332-apcs-apps-global
>> - const: qcom,ipq6018-apcs-apps-global
>>
>> and drop the next patch
>
> Is it still ok even if the two devices are not fully compatible (iow,
> using different PLL types)?
Ignore my question, I mixed the A53 and APCS clocks.
--
With best wishes
Dmitry
On 2/2/2023 9:05 PM, Krzysztof Kozlowski wrote:
> On 02/02/2023 15:52, Kathiravan T wrote:
>> Add the mailbox compatible for the IPQ5332 SoC.
>>
>> Signed-off-by: Kathiravan T <[email protected]>
>> ---
>> .../devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
>> index 943f9472ae10..8d8cd1bbe67e 100644
>> --- a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
>> +++ b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
>> @@ -18,6 +18,7 @@ properties:
>> oneOf:
> - items:
> - enum:
> - qcom,ipq5332-apcs-apps-global
> - const: qcom,ipq6018-apcs-apps-global
>
> and drop the next patch
Hi Krzysztof,
I'm planning to post the V2 of this series. How do you want me to
proceed? I see you posted separate series[1]. May be I can follow the
suggestion which you shared above, starting with IPQ?
[1]
https://lore.kernel.org/linux-arm-msm/[email protected]/
Thanks,
>
>
> Best regards,
> Krzysztof
>
On 06/02/2023 10:12, Kathiravan T wrote:
>
> On 2/2/2023 9:05 PM, Krzysztof Kozlowski wrote:
>> On 02/02/2023 15:52, Kathiravan T wrote:
>>> Add the mailbox compatible for the IPQ5332 SoC.
>>>
>>> Signed-off-by: Kathiravan T <[email protected]>
>>> ---
>>> .../devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
>>> index 943f9472ae10..8d8cd1bbe67e 100644
>>> --- a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
>>> +++ b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
>>> @@ -18,6 +18,7 @@ properties:
>>> oneOf:
>> - items:
>> - enum:
>> - qcom,ipq5332-apcs-apps-global
>> - const: qcom,ipq6018-apcs-apps-global
>>
>> and drop the next patch
>
>
> Hi Krzysztof,
>
> I'm planning to post the V2 of this series. How do you want me to
> proceed? I see you posted separate series[1]. May be I can follow the
> suggestion which you shared above, starting with IPQ?
My series need rebasing on top of Dmitry's fixes, so I propose you just
send patch working only for ipq6018, like I wrote above.
Best regards,
Krzysztof