The CPU on Qualcomm's IPQ6018 devices are primarily fed by A53 PLL and XO,
these are connected to a clock mux and enable block.
This patch series adds support for these clocks and inturn enables clocks
required for CPU freq.
[V3]
* Fixed dt binding check error in patch2
dt-bindings: clock: Add YAML schemas for QCOM A53 PLL
[V2]
* Restructred the patch series as there are two different HW blocks,
the mux and enable belongs to the apcs block and PLL has a separate HW
block.
* Converted qcom mailbox and qcom a53 pll documentation to yaml.
* Addressed review comments from Stephen, Rob and Sibi where it is applicable.
* Changed this cover letter to state the purpose of this patch series
Sivaprakash Murugesan (8):
dt-bindings: mailbox: Add YAML schemas for QCOM APCS global block
dt-bindings: clock: Add YAML schemas for QCOM A53 PLL
clk: qcom: Add A53 PLL support for ipq6018 devices
clk: qcom: Add DT bindings for ipq6018 apss clock controller
clk: qcom: Add ipq apss clock controller
dt-bindings: mailbox: Add dt-bindings for ipq6018 apcs global block
mailbox: qcom: Add ipq6018 apcs compatible
arm64: dts: ipq6018: Add a53 pll and apcs clock
.../devicetree/bindings/clock/qcom,a53pll.txt | 22 ----
.../devicetree/bindings/clock/qcom,a53pll.yaml | 60 +++++++++
.../bindings/mailbox/qcom,apcs-kpss-global.txt | 88 -------------
.../bindings/mailbox/qcom,apcs-kpss-global.yaml | 101 +++++++++++++++
arch/arm64/boot/dts/qcom/ipq6018.dtsi | 16 ++-
drivers/clk/qcom/Kconfig | 10 ++
drivers/clk/qcom/Makefile | 1 +
drivers/clk/qcom/a53-pll.c | 136 +++++++++++++++++----
drivers/clk/qcom/apss-ipq.c | 107 ++++++++++++++++
drivers/mailbox/qcom-apcs-ipc-mailbox.c | 22 ++--
include/dt-bindings/clock/qcom,apss-ipq.h | 12 ++
11 files changed, 430 insertions(+), 145 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/clock/qcom,a53pll.txt
create mode 100644 Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
delete mode 100644 Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt
create mode 100644 Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
create mode 100644 drivers/clk/qcom/apss-ipq.c
create mode 100644 include/dt-bindings/clock/qcom,apss-ipq.h
--
2.7.4
The CPUs on Qualcomm IPQ6018 platform is primarily clocked by A53 PLL.
This patch adds support for the A53 PLL on IPQ6018 devices which can
support CPU frequencies above 1Ghz.
Signed-off-by: Sivaprakash Murugesan <[email protected]>
---
drivers/clk/qcom/a53-pll.c | 136 ++++++++++++++++++++++++++++++++++++---------
1 file changed, 111 insertions(+), 25 deletions(-)
diff --git a/drivers/clk/qcom/a53-pll.c b/drivers/clk/qcom/a53-pll.c
index 45cfc57..a95351c 100644
--- a/drivers/clk/qcom/a53-pll.c
+++ b/drivers/clk/qcom/a53-pll.c
@@ -11,11 +11,40 @@
#include <linux/platform_device.h>
#include <linux/regmap.h>
#include <linux/module.h>
+#include <linux/of_device.h>
#include "clk-pll.h"
#include "clk-regmap.h"
+#include "clk-alpha-pll.h"
-static const struct pll_freq_tbl a53pll_freq[] = {
+struct a53_alpha_pll {
+ struct alpha_pll_config *pll_config;
+ struct clk_alpha_pll *pll;
+};
+
+union a53pll {
+ struct clk_pll *pll;
+ struct a53_alpha_pll alpha_pll;
+};
+
+struct a53pll_data {
+#define PLL_IS_ALPHA BIT(0)
+ u8 flags;
+ union a53pll a53pll;
+};
+
+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 struct pll_freq_tbl msm8996_a53pll_freq[] = {
{ 998400000, 52, 0x0, 0x1, 0 },
{ 1094400000, 57, 0x0, 0x1, 0 },
{ 1152000000, 62, 0x0, 0x1, 0 },
@@ -26,6 +55,64 @@ static const struct pll_freq_tbl a53pll_freq[] = {
{ }
};
+static struct clk_pll msm8996_pll = {
+ .mode_reg = 0x0,
+ .l_reg = 0x04,
+ .m_reg = 0x08,
+ .n_reg = 0x0c,
+ .config_reg = 0x14,
+ .status_reg = 0x1c,
+ .status_bit = 16,
+ .freq_tbl = msm8996_a53pll_freq,
+ .clkr.hw.init = &(struct clk_init_data){
+ .name = "a53pll",
+ .flags = CLK_IS_CRITICAL,
+ .parent_data = &(const struct clk_parent_data){
+ .fw_name = "xo",
+ .name = "xo",
+ },
+ .num_parents = 1,
+ .ops = &clk_pll_sr2_ops,
+ },
+};
+
+static struct clk_alpha_pll ipq6018_pll = {
+ .offset = 0x0,
+ .regs = ipq_pll_offsets,
+ .flags = SUPPORTS_DYNAMIC_UPDATE,
+ .clkr = {
+ .enable_reg = 0x0,
+ .enable_mask = BIT(0),
+ .hw.init = &(struct clk_init_data){
+ .name = "a53pll",
+ .flags = CLK_IS_CRITICAL,
+ .parent_data = &(const struct clk_parent_data){
+ .fw_name = "xo",
+ },
+ .num_parents = 1,
+ .ops = &clk_alpha_pll_huayra_ops,
+ },
+ },
+};
+
+static struct alpha_pll_config ipq6018_pll_config = {
+ .l = 0x37,
+ .config_ctl_val = 0x04141200,
+ .config_ctl_hi_val = 0x0,
+ .early_output_mask = BIT(3),
+ .main_output_mask = BIT(0),
+};
+
+static struct a53pll_data msm8996pll_data = {
+ .a53pll.pll = &msm8996_pll,
+};
+
+static struct a53pll_data ipq6018pll_data = {
+ .flags = PLL_IS_ALPHA,
+ .a53pll.alpha_pll.pll = &ipq6018_pll,
+ .a53pll.alpha_pll.pll_config = &ipq6018_pll_config,
+};
+
static const struct regmap_config a53pll_regmap_config = {
.reg_bits = 32,
.reg_stride = 4,
@@ -39,14 +126,16 @@ static int qcom_a53pll_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct regmap *regmap;
struct resource *res;
- struct clk_pll *pll;
+ const struct a53pll_data *pll_data;
+ struct clk_regmap *clkr;
void __iomem *base;
- struct clk_init_data init = { };
int ret;
- pll = devm_kzalloc(dev, sizeof(*pll), GFP_KERNEL);
- if (!pll)
- return -ENOMEM;
+ pll_data = of_device_get_match_data(dev);
+ if (!pll_data) {
+ dev_err(dev, "failed to get platform data\n");
+ return -ENODEV;
+ }
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
base = devm_ioremap_resource(dev, res);
@@ -57,30 +146,26 @@ static int qcom_a53pll_probe(struct platform_device *pdev)
if (IS_ERR(regmap))
return PTR_ERR(regmap);
- pll->l_reg = 0x04;
- pll->m_reg = 0x08;
- pll->n_reg = 0x0c;
- pll->config_reg = 0x14;
- pll->mode_reg = 0x00;
- pll->status_reg = 0x1c;
- pll->status_bit = 16;
- pll->freq_tbl = a53pll_freq;
-
- init.name = "a53pll";
- init.parent_names = (const char *[]){ "xo" };
- init.num_parents = 1;
- init.ops = &clk_pll_sr2_ops;
- init.flags = CLK_IS_CRITICAL;
- pll->clkr.hw.init = &init;
-
- ret = devm_clk_register_regmap(dev, &pll->clkr);
+ if (pll_data->flags & PLL_IS_ALPHA) {
+ struct clk_alpha_pll *alpha_pll =
+ pll_data->a53pll.alpha_pll.pll;
+ struct alpha_pll_config *alpha_pll_config =
+ pll_data->a53pll.alpha_pll.pll_config;
+
+ clk_alpha_pll_configure(alpha_pll, regmap, alpha_pll_config);
+ clkr = &pll_data->a53pll.alpha_pll.pll->clkr;
+ } else {
+ clkr = &pll_data->a53pll.pll->clkr;
+ }
+
+ ret = devm_clk_register_regmap(dev, clkr);
if (ret) {
dev_err(dev, "failed to register regmap clock: %d\n", ret);
return ret;
}
ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get,
- &pll->clkr.hw);
+ &clkr->hw);
if (ret) {
dev_err(dev, "failed to add clock provider: %d\n", ret);
return ret;
@@ -90,7 +175,8 @@ static int qcom_a53pll_probe(struct platform_device *pdev)
}
static const struct of_device_id qcom_a53pll_match_table[] = {
- { .compatible = "qcom,msm8916-a53pll" },
+ { .compatible = "qcom,msm8916-a53pll", .data = &msm8996pll_data},
+ { .compatible = "qcom,ipq6018-a53pll", .data = &ipq6018pll_data},
{ }
};
--
2.7.4
add support for a53 pll and apcs clock.
Signed-off-by: Sivaprakash Murugesan <[email protected]>
---
arch/arm64/boot/dts/qcom/ipq6018.dtsi | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
index 1aa8d85..3c2d91a 100644
--- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
@@ -294,12 +294,22 @@
};
apcs_glb: mailbox@b111000 {
- compatible = "qcom,ipq8074-apcs-apps-global";
- reg = <0x0b111000 0xc>;
-
+ compatible = "qcom,ipq6018-apcs-apps-global";
+ reg = <0x0b111000 0x1000>;
+ #clock-cells = <1>;
+ clocks = <&a53pll>, <&xo>;
+ clock-names = "pll", "xo";
#mbox-cells = <1>;
};
+ a53pll: clock@b116000 {
+ compatible = "qcom,ipq6018-a53pll";
+ reg = <0x0b116000 0x40>;
+ #clock-cells = <0>;
+ clocks = <&xo>;
+ clock-names = "xo";
+ };
+
timer {
compatible = "arm,armv8-timer";
interrupts = <GIC_PPI 2 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
--
2.7.4
The Qualcomm ipq6018 has apcs block, add compatible for the same.
Also, the apcs provides a clock controller functionality similar
to msm8916 but the clock driver is different.
Create a child platform device based on the apcs compatible for the
clock controller functionality.
Signed-off-by: Sivaprakash Murugesan <[email protected]>
---
drivers/mailbox/qcom-apcs-ipc-mailbox.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/mailbox/qcom-apcs-ipc-mailbox.c b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
index eeebafd..19f6ba1 100644
--- a/drivers/mailbox/qcom-apcs-ipc-mailbox.c
+++ b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
@@ -45,6 +45,16 @@ static const struct mbox_chan_ops qcom_apcs_ipc_ops = {
.send_data = qcom_apcs_ipc_send_data,
};
+static const struct of_device_id apcs_clk_match_table[] = {
+ { .compatible = "qcom,msm8916-apcs-kpss-global",
+ .data = "qcom-apcs-msm8916-clk", },
+ { .compatible = "qcom,qcs404-apcs-apps-global",
+ .data = "qcom-apcs-msm8916-clk", },
+ { .compatible = "qcom,ipq6018-apcs-apps-global",
+ .data = "qcom,apss-ipq-clk", },
+ {}
+};
+
static int qcom_apcs_ipc_probe(struct platform_device *pdev)
{
struct qcom_apcs_ipc *apcs;
@@ -54,11 +64,7 @@ static int qcom_apcs_ipc_probe(struct platform_device *pdev)
void __iomem *base;
unsigned long i;
int ret;
- const struct of_device_id apcs_clk_match_table[] = {
- { .compatible = "qcom,msm8916-apcs-kpss-global", },
- { .compatible = "qcom,qcs404-apcs-apps-global", },
- {}
- };
+ const struct of_device_id *clk_device;
apcs = devm_kzalloc(&pdev->dev, sizeof(*apcs), GFP_KERNEL);
if (!apcs)
@@ -93,9 +99,10 @@ static int qcom_apcs_ipc_probe(struct platform_device *pdev)
return ret;
}
- if (of_match_device(apcs_clk_match_table, &pdev->dev)) {
+ clk_device = of_match_device(apcs_clk_match_table, &pdev->dev);
+ if (clk_device) {
apcs->clk = platform_device_register_data(&pdev->dev,
- "qcom-apcs-msm8916-clk",
+ (const char *)clk_device->data,
PLATFORM_DEVID_NONE,
NULL, 0);
if (IS_ERR(apcs->clk))
@@ -127,6 +134,7 @@ static const struct of_device_id qcom_apcs_ipc_of_match[] = {
{ .compatible = "qcom,sdm845-apss-shared", .data = (void *)12 },
{ .compatible = "qcom,sm8150-apss-shared", .data = (void *)12 },
{ .compatible = "qcom,ipq8074-apcs-apps-global", .data = (void *)8 },
+ { .compatible = "qcom,ipq6018-apcs-apps-global", .data = (void *)8 },
{}
};
MODULE_DEVICE_TABLE(of, qcom_apcs_ipc_of_match);
--
2.7.4
The CPU on Qualcomm's IPQ platform devices are clocked primarily by a
PLL and xo which are connected to a mux and enable block, This patch adds
support for the mux and the enable.
Signed-off-by: Sivaprakash Murugesan <[email protected]>
---
drivers/clk/qcom/Kconfig | 10 +++++
drivers/clk/qcom/Makefile | 1 +
drivers/clk/qcom/apss-ipq.c | 107 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 118 insertions(+)
create mode 100644 drivers/clk/qcom/apss-ipq.c
diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index 15cdcdc..8573f2e 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -89,6 +89,16 @@ config APQ_MMCC_8084
Say Y if you want to support multimedia devices such as display,
graphics, video encode/decode, camera, etc.
+config IPQ_APSS
+ tristate "IPQ APSS Clock Controller"
+ default N
+ help
+ Support for APSS clock controller on ipq platform devices. The
+ APSS clock controller manages the Mux and enable block that feeds the
+ CPUs.
+ Say Y if you want to support CPU frequency scaling on
+ ipq based devices.
+
config IPQ_GCC_4019
tristate "IPQ4019 Global Clock Controller"
help
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index 656a87e..1e4b296 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -19,6 +19,7 @@ clk-qcom-$(CONFIG_QCOM_GDSC) += gdsc.o
# Keep alphabetically sorted by config
obj-$(CONFIG_APQ_GCC_8084) += gcc-apq8084.o
obj-$(CONFIG_APQ_MMCC_8084) += mmcc-apq8084.o
+obj-$(CONFIG_IPQ_APSS) += apss-ipq.o
obj-$(CONFIG_IPQ_GCC_4019) += gcc-ipq4019.o
obj-$(CONFIG_IPQ_GCC_6018) += gcc-ipq6018.o
obj-$(CONFIG_IPQ_GCC_806X) += gcc-ipq806x.o
diff --git a/drivers/clk/qcom/apss-ipq.c b/drivers/clk/qcom/apss-ipq.c
new file mode 100644
index 0000000..a37cd98
--- /dev/null
+++ b/drivers/clk/qcom/apss-ipq.c
@@ -0,0 +1,107 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/kernel.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/clk-provider.h>
+#include <linux/regmap.h>
+#include <linux/module.h>
+
+#include <dt-bindings/clock/qcom,apss-ipq.h>
+
+#include "common.h"
+#include "clk-regmap.h"
+#include "clk-branch.h"
+#include "clk-alpha-pll.h"
+#include "clk-regmap-mux.h"
+
+enum {
+ P_XO,
+ P_APSS_PLL_EARLY,
+};
+
+static const struct clk_parent_data parents_apcs_alias0_clk_src[] = {
+ { .fw_name = "xo" },
+ { .fw_name = "pll" },
+};
+
+static const struct parent_map parents_apcs_alias0_clk_src_map[] = {
+ { P_XO, 0 },
+ { P_APSS_PLL_EARLY, 5 },
+};
+
+static struct clk_regmap_mux apcs_alias0_clk_src = {
+ .reg = 0x0050,
+ .width = 3,
+ .shift = 7,
+ .parent_map = parents_apcs_alias0_clk_src_map,
+ .clkr.hw.init = &(struct clk_init_data){
+ .name = "apcs_alias0_clk_src",
+ .parent_data = parents_apcs_alias0_clk_src,
+ .num_parents = 2,
+ .ops = &clk_regmap_mux_closest_ops,
+ .flags = CLK_SET_RATE_PARENT,
+ },
+};
+
+/*required for cpufreq*/
+static struct clk_branch apcs_alias0_core_clk = {
+ .halt_reg = 0x0058,
+ .clkr = {
+ .enable_reg = 0x0058,
+ .enable_mask = BIT(0),
+ .hw.init = &(struct clk_init_data){
+ .name = "apcs_alias0_core_clk",
+ .parent_hws = (const struct clk_hw *[]){
+ &apcs_alias0_clk_src.clkr.hw },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
+static const struct regmap_config apss_ipq_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .max_register = 0x1000,
+ .fast_io = true,
+};
+
+static struct clk_regmap *apss_ipq_clks[] = {
+ [APCS_ALIAS0_CLK_SRC] = &apcs_alias0_clk_src.clkr,
+ [APCS_ALIAS0_CORE_CLK] = &apcs_alias0_core_clk.clkr,
+};
+
+static const struct qcom_cc_desc apss_ipq_desc = {
+ .config = &apss_ipq_regmap_config,
+ .clks = apss_ipq_clks,
+ .num_clks = ARRAY_SIZE(apss_ipq_clks),
+};
+
+static int apss_ipq_probe(struct platform_device *pdev)
+{
+ struct regmap *regmap;
+
+ regmap = dev_get_regmap(pdev->dev.parent, NULL);
+ if (IS_ERR(regmap))
+ return PTR_ERR(regmap);
+
+ return qcom_cc_really_probe(pdev, &apss_ipq_desc, regmap);
+}
+
+static struct platform_driver apss_ipq_driver = {
+ .probe = apss_ipq_probe,
+ .driver = {
+ .name = "qcom,apss-ipq-clk",
+ },
+};
+
+module_platform_driver(apss_ipq_driver);
+
+MODULE_DESCRIPTION("QCOM APSS IPQ CLK Driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4
Add dt-bindings for ipq6018 mailbox driver
Signed-off-by: Sivaprakash Murugesan <[email protected]>
---
.../bindings/mailbox/qcom,apcs-kpss-global.yaml | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
index b46474b..07180c0 100644
--- a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
+++ b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
@@ -16,6 +16,7 @@ maintainers:
properties:
compatible:
enum:
+ - qcom,ipq6018-apcs-apps-global
- qcom,ipq8074-apcs-apps-global
- qcom,msm8916-apcs-kpss-global
- qcom,msm8996-apcs-hmss-global
@@ -36,7 +37,7 @@ properties:
const: 1
'#clock-cells':
- const: 0
+ enum: [ 0, 1 ]
clock-names:
description:
@@ -45,7 +46,7 @@ properties:
maxItems: 2
items:
- const: pll
- - const: aux
+ - enum: [ aux, xo ]
required:
- compatible
@@ -86,3 +87,15 @@ examples:
clock-names = "pll", "aux";
#clock-cells = <0>;
};
+
+ # Example apcs with ipq6018
+ - |
+ #include "dt-bindings/clock/qcom,apss-ipq.h"
+ apcs_ipq: mailbox@b111000 {
+ compatible = "qcom,ipq6018-apcs-apps-global";
+ reg = <0x0b111000 0x1000>;
+ #clock-cells = <1>;
+ clocks = <&a53pll>, <&xo>;
+ clock-names = "pll", "xo";
+ #mbox-cells = <1>;
+ };
--
2.7.4
On Tue, 14 Apr 2020 08:25:20 +0530, Sivaprakash Murugesan wrote:
> Add dt-bindings for ipq6018 mailbox driver
>
> Signed-off-by: Sivaprakash Murugesan <[email protected]>
> ---
> .../bindings/mailbox/qcom,apcs-kpss-global.yaml | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
Reviewed-by: Rob Herring <[email protected]>
Quoting Sivaprakash Murugesan (2020-04-13 19:55:17)
> The CPUs on Qualcomm IPQ6018 platform is primarily clocked by A53 PLL.
> This patch adds support for the A53 PLL on IPQ6018 devices which can
> support CPU frequencies above 1Ghz.
>
> Signed-off-by: Sivaprakash Murugesan <[email protected]>
> ---
> drivers/clk/qcom/a53-pll.c | 136 ++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 111 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/clk/qcom/a53-pll.c b/drivers/clk/qcom/a53-pll.c
> index 45cfc57..a95351c 100644
> --- a/drivers/clk/qcom/a53-pll.c
> +++ b/drivers/clk/qcom/a53-pll.c
> @@ -11,11 +11,40 @@
> #include <linux/platform_device.h>
> #include <linux/regmap.h>
> #include <linux/module.h>
> +#include <linux/of_device.h>
Why does this driver need to change to use of_device APIs?
>
> #include "clk-pll.h"
> #include "clk-regmap.h"
> +#include "clk-alpha-pll.h"
>
> -static const struct pll_freq_tbl a53pll_freq[] = {
> +struct a53_alpha_pll {
> + struct alpha_pll_config *pll_config;
> + struct clk_alpha_pll *pll;
> +};
> +
> +union a53pll {
> + struct clk_pll *pll;
> + struct a53_alpha_pll alpha_pll;
> +};
> +
> +struct a53pll_data {
> +#define PLL_IS_ALPHA BIT(0)
> + u8 flags;
> + union a53pll a53pll;
Why is there a union? Can't we have different clk ops for the two types
of PLLs and then use container_of to get it from the clk ops?
> +};
> +
> +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 struct pll_freq_tbl msm8996_a53pll_freq[] = {
> { 998400000, 52, 0x0, 0x1, 0 },
> { 1094400000, 57, 0x0, 0x1, 0 },
> { 1152000000, 62, 0x0, 0x1, 0 },
> @@ -26,6 +55,64 @@ static const struct pll_freq_tbl a53pll_freq[] = {
> { }
> };
>
> +static struct clk_pll msm8996_pll = {
> + .mode_reg = 0x0,
> + .l_reg = 0x04,
> + .m_reg = 0x08,
> + .n_reg = 0x0c,
> + .config_reg = 0x14,
> + .status_reg = 0x1c,
> + .status_bit = 16,
> + .freq_tbl = msm8996_a53pll_freq,
> + .clkr.hw.init = &(struct clk_init_data){
> + .name = "a53pll",
> + .flags = CLK_IS_CRITICAL,
> + .parent_data = &(const struct clk_parent_data){
> + .fw_name = "xo",
> + .name = "xo",
> + },
> + .num_parents = 1,
> + .ops = &clk_pll_sr2_ops,
> + },
> +};
> +
> +static struct clk_alpha_pll ipq6018_pll = {
> + .offset = 0x0,
> + .regs = ipq_pll_offsets,
> + .flags = SUPPORTS_DYNAMIC_UPDATE,
> + .clkr = {
> + .enable_reg = 0x0,
> + .enable_mask = BIT(0),
> + .hw.init = &(struct clk_init_data){
> + .name = "a53pll",
> + .flags = CLK_IS_CRITICAL,
> + .parent_data = &(const struct clk_parent_data){
> + .fw_name = "xo",
> + },
> + .num_parents = 1,
> + .ops = &clk_alpha_pll_huayra_ops,
> + },
> + },
> +};
> +
> +static struct alpha_pll_config ipq6018_pll_config = {
Can this be const?
> + .l = 0x37,
> + .config_ctl_val = 0x04141200,
> + .config_ctl_hi_val = 0x0,
> + .early_output_mask = BIT(3),
> + .main_output_mask = BIT(0),
> +};
> +
> +static struct a53pll_data msm8996pll_data = {
> + .a53pll.pll = &msm8996_pll,
> +};
> +
> +static struct a53pll_data ipq6018pll_data = {
> + .flags = PLL_IS_ALPHA,
> + .a53pll.alpha_pll.pll = &ipq6018_pll,
> + .a53pll.alpha_pll.pll_config = &ipq6018_pll_config,
> +};
> +
> static const struct regmap_config a53pll_regmap_config = {
> .reg_bits = 32,
> .reg_stride = 4,
> @@ -39,14 +126,16 @@ static int qcom_a53pll_probe(struct platform_device *pdev)
> struct device *dev = &pdev->dev;
> struct regmap *regmap;
> struct resource *res;
> - struct clk_pll *pll;
> + const struct a53pll_data *pll_data;
> + struct clk_regmap *clkr;
> void __iomem *base;
> - struct clk_init_data init = { };
> int ret;
>
> - pll = devm_kzalloc(dev, sizeof(*pll), GFP_KERNEL);
> - if (!pll)
> - return -ENOMEM;
> + pll_data = of_device_get_match_data(dev);
Use device_get_match_data() please.
> + if (!pll_data) {
> + dev_err(dev, "failed to get platform data\n");
No error message please.
> + return -ENODEV;
> + }
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> base = devm_ioremap_resource(dev, res);
> @@ -57,30 +146,26 @@ static int qcom_a53pll_probe(struct platform_device *pdev)
> if (IS_ERR(regmap))
> return PTR_ERR(regmap);
>
> - pll->l_reg = 0x04;
> - pll->m_reg = 0x08;
> - pll->n_reg = 0x0c;
> - pll->config_reg = 0x14;
> - pll->mode_reg = 0x00;
> - pll->status_reg = 0x1c;
> - pll->status_bit = 16;
> - pll->freq_tbl = a53pll_freq;
> -
> - init.name = "a53pll";
> - init.parent_names = (const char *[]){ "xo" };
> - init.num_parents = 1;
> - init.ops = &clk_pll_sr2_ops;
> - init.flags = CLK_IS_CRITICAL;
Please document why a clk is critical.
> - pll->clkr.hw.init = &init;
> -
> - ret = devm_clk_register_regmap(dev, &pll->clkr);
> + if (pll_data->flags & PLL_IS_ALPHA) {
> + struct clk_alpha_pll *alpha_pll =
> + pll_data->a53pll.alpha_pll.pll;
> + struct alpha_pll_config *alpha_pll_config =
> + pll_data->a53pll.alpha_pll.pll_config;
> +
> + clk_alpha_pll_configure(alpha_pll, regmap, alpha_pll_config);
> + clkr = &pll_data->a53pll.alpha_pll.pll->clkr;
> + } else {
> + clkr = &pll_data->a53pll.pll->clkr;
> + }
Sorry, the design is confusing.
Quoting Sivaprakash Murugesan (2020-04-13 19:55:19)
> The CPU on Qualcomm's IPQ platform devices are clocked primarily by a
> PLL and xo which are connected to a mux and enable block, This patch adds
The comma should be a period? Don't write "This patch" in commit text.
> support for the mux and the enable.
>
> Signed-off-by: Sivaprakash Murugesan <[email protected]>
> ---
> drivers/clk/qcom/Kconfig | 10 +++++
> drivers/clk/qcom/Makefile | 1 +
> drivers/clk/qcom/apss-ipq.c | 107 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 118 insertions(+)
> create mode 100644 drivers/clk/qcom/apss-ipq.c
>
> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> index 15cdcdc..8573f2e 100644
> --- a/drivers/clk/qcom/Kconfig
> +++ b/drivers/clk/qcom/Kconfig
> @@ -89,6 +89,16 @@ config APQ_MMCC_8084
> Say Y if you want to support multimedia devices such as display,
> graphics, video encode/decode, camera, etc.
>
> +config IPQ_APSS
> + tristate "IPQ APSS Clock Controller"
> + default N
Drop this, it's already the default.
> + help
> + Support for APSS clock controller on ipq platform devices. The
Maybe say "IPQ platforms" and drop the devices part?
> + APSS clock controller manages the Mux and enable block that feeds the
> + CPUs.
> + Say Y if you want to support CPU frequency scaling on
> + ipq based devices.
> +
> config IPQ_GCC_4019
> tristate "IPQ4019 Global Clock Controller"
> help
> diff --git a/drivers/clk/qcom/apss-ipq.c b/drivers/clk/qcom/apss-ipq.c
> new file mode 100644
> index 0000000..a37cd98
> --- /dev/null
> +++ b/drivers/clk/qcom/apss-ipq.c
> @@ -0,0 +1,107 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/err.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk-provider.h>
> +#include <linux/regmap.h>
> +#include <linux/module.h>
> +
> +#include <dt-bindings/clock/qcom,apss-ipq.h>
> +
> +#include "common.h"
> +#include "clk-regmap.h"
> +#include "clk-branch.h"
> +#include "clk-alpha-pll.h"
> +#include "clk-regmap-mux.h"
> +
> +enum {
> + P_XO,
> + P_APSS_PLL_EARLY,
> +};
> +
> +static const struct clk_parent_data parents_apcs_alias0_clk_src[] = {
> + { .fw_name = "xo" },
> + { .fw_name = "pll" },
> +};
> +
> +static const struct parent_map parents_apcs_alias0_clk_src_map[] = {
> + { P_XO, 0 },
> + { P_APSS_PLL_EARLY, 5 },
> +};
> +
> +static struct clk_regmap_mux apcs_alias0_clk_src = {
> + .reg = 0x0050,
> + .width = 3,
> + .shift = 7,
> + .parent_map = parents_apcs_alias0_clk_src_map,
> + .clkr.hw.init = &(struct clk_init_data){
> + .name = "apcs_alias0_clk_src",
> + .parent_data = parents_apcs_alias0_clk_src,
> + .num_parents = 2,
> + .ops = &clk_regmap_mux_closest_ops,
> + .flags = CLK_SET_RATE_PARENT,
> + },
> +};
> +
> +/*required for cpufreq*/
This comment doesn't help in understanding.
> +static struct clk_branch apcs_alias0_core_clk = {
> + .halt_reg = 0x0058,
> + .clkr = {
> + .enable_reg = 0x0058,
> + .enable_mask = BIT(0),
> + .hw.init = &(struct clk_init_data){
> + .name = "apcs_alias0_core_clk",
> + .parent_hws = (const struct clk_hw *[]){
> + &apcs_alias0_clk_src.clkr.hw },
> + .num_parents = 1,
> + .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
Please comment why the clk is critical.
> + .ops = &clk_branch2_ops,
> + },
> + },
> +};
> +
> +static const struct regmap_config apss_ipq_regmap_config = {
> + .reg_bits = 32,
> + .reg_stride = 4,
> + .val_bits = 32,
> + .max_register = 0x1000,
> + .fast_io = true,
> +};
> +
> +static struct clk_regmap *apss_ipq_clks[] = {
> + [APCS_ALIAS0_CLK_SRC] = &apcs_alias0_clk_src.clkr,
> + [APCS_ALIAS0_CORE_CLK] = &apcs_alias0_core_clk.clkr,
> +};
> +
> +static const struct qcom_cc_desc apss_ipq_desc = {
> + .config = &apss_ipq_regmap_config,
> + .clks = apss_ipq_clks,
> + .num_clks = ARRAY_SIZE(apss_ipq_clks),
> +};
> +
> +static int apss_ipq_probe(struct platform_device *pdev)
> +{
> + struct regmap *regmap;
> +
> + regmap = dev_get_regmap(pdev->dev.parent, NULL);
> + if (IS_ERR(regmap))
> + return PTR_ERR(regmap);
> +
> + return qcom_cc_really_probe(pdev, &apss_ipq_desc, regmap);
What is this a child of?
> +}
> +
> +static struct platform_driver apss_ipq_driver = {
> + .probe = apss_ipq_probe,
> + .driver = {
> + .name = "qcom,apss-ipq-clk",
> + },
> +};
Hi Stephen,
On 4/22/2020 2:30 PM, Stephen Boyd wrote:
> Quoting Sivaprakash Murugesan (2020-04-13 19:55:17)
>> The CPUs on Qualcomm IPQ6018 platform is primarily clocked by A53 PLL.
>> This patch adds support for the A53 PLL on IPQ6018 devices which can
>> support CPU frequencies above 1Ghz.
>>
>> Signed-off-by: Sivaprakash Murugesan <[email protected]>
>> ---
>> drivers/clk/qcom/a53-pll.c | 136 ++++++++++++++++++++++++++++++++++++---------
>> 1 file changed, 111 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/a53-pll.c b/drivers/clk/qcom/a53-pll.c
>> index 45cfc57..a95351c 100644
>> --- a/drivers/clk/qcom/a53-pll.c
>> +++ b/drivers/clk/qcom/a53-pll.c
>> @@ -11,11 +11,40 @@
>> #include <linux/platform_device.h>
>> #include <linux/regmap.h>
>> #include <linux/module.h>
>> +#include <linux/of_device.h>
> Why does this driver need to change to use of_device APIs?
we can use devm APIs and avoid this in next patch.
>
>>
>> #include "clk-pll.h"
>> #include "clk-regmap.h"
>> +#include "clk-alpha-pll.h"
>>
>> -static const struct pll_freq_tbl a53pll_freq[] = {
>> +struct a53_alpha_pll {
>> + struct alpha_pll_config *pll_config;
>> + struct clk_alpha_pll *pll;
>> +};
>> +
>> +union a53pll {
>> + struct clk_pll *pll;
>> + struct a53_alpha_pll alpha_pll;
>> +};
>> +
>> +struct a53pll_data {
>> +#define PLL_IS_ALPHA BIT(0)
>> + u8 flags;
>> + union a53pll a53pll;
> Why is there a union? Can't we have different clk ops for the two types
> of PLLs and then use container_of to get it from the clk ops?
ok.
>
>> +};
>> +
>> +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 struct pll_freq_tbl msm8996_a53pll_freq[] = {
>> { 998400000, 52, 0x0, 0x1, 0 },
>> { 1094400000, 57, 0x0, 0x1, 0 },
>> { 1152000000, 62, 0x0, 0x1, 0 },
>> @@ -26,6 +55,64 @@ static const struct pll_freq_tbl a53pll_freq[] = {
>> { }
>> };
>>
>> +static struct clk_pll msm8996_pll = {
>> + .mode_reg = 0x0,
>> + .l_reg = 0x04,
>> + .m_reg = 0x08,
>> + .n_reg = 0x0c,
>> + .config_reg = 0x14,
>> + .status_reg = 0x1c,
>> + .status_bit = 16,
>> + .freq_tbl = msm8996_a53pll_freq,
>> + .clkr.hw.init = &(struct clk_init_data){
>> + .name = "a53pll",
>> + .flags = CLK_IS_CRITICAL,
>> + .parent_data = &(const struct clk_parent_data){
>> + .fw_name = "xo",
>> + .name = "xo",
>> + },
>> + .num_parents = 1,
>> + .ops = &clk_pll_sr2_ops,
>> + },
>> +};
>> +
>> +static struct clk_alpha_pll ipq6018_pll = {
>> + .offset = 0x0,
>> + .regs = ipq_pll_offsets,
>> + .flags = SUPPORTS_DYNAMIC_UPDATE,
>> + .clkr = {
>> + .enable_reg = 0x0,
>> + .enable_mask = BIT(0),
>> + .hw.init = &(struct clk_init_data){
>> + .name = "a53pll",
>> + .flags = CLK_IS_CRITICAL,
>> + .parent_data = &(const struct clk_parent_data){
>> + .fw_name = "xo",
>> + },
>> + .num_parents = 1,
>> + .ops = &clk_alpha_pll_huayra_ops,
>> + },
>> + },
>> +};
>> +
>> +static struct alpha_pll_config ipq6018_pll_config = {
> Can this be const?
yeah it can be. will fix up in next patch.
>
>> + .l = 0x37,
>> + .config_ctl_val = 0x04141200,
>> + .config_ctl_hi_val = 0x0,
>> + .early_output_mask = BIT(3),
>> + .main_output_mask = BIT(0),
>> +};
>> +
>> +static struct a53pll_data msm8996pll_data = {
>> + .a53pll.pll = &msm8996_pll,
>> +};
>> +
>> +static struct a53pll_data ipq6018pll_data = {
>> + .flags = PLL_IS_ALPHA,
>> + .a53pll.alpha_pll.pll = &ipq6018_pll,
>> + .a53pll.alpha_pll.pll_config = &ipq6018_pll_config,
>> +};
>> +
>> static const struct regmap_config a53pll_regmap_config = {
>> .reg_bits = 32,
>> .reg_stride = 4,
>> @@ -39,14 +126,16 @@ static int qcom_a53pll_probe(struct platform_device *pdev)
>> struct device *dev = &pdev->dev;
>> struct regmap *regmap;
>> struct resource *res;
>> - struct clk_pll *pll;
>> + const struct a53pll_data *pll_data;
>> + struct clk_regmap *clkr;
>> void __iomem *base;
>> - struct clk_init_data init = { };
>> int ret;
>>
>> - pll = devm_kzalloc(dev, sizeof(*pll), GFP_KERNEL);
>> - if (!pll)
>> - return -ENOMEM;
>> + pll_data = of_device_get_match_data(dev);
> Use device_get_match_data() please.
ok.
>
>> + if (!pll_data) {
>> + dev_err(dev, "failed to get platform data\n");
> No error message please.
ok.
>
>> + return -ENODEV;
>> + }
>>
>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> base = devm_ioremap_resource(dev, res);
>> @@ -57,30 +146,26 @@ static int qcom_a53pll_probe(struct platform_device *pdev)
>> if (IS_ERR(regmap))
>> return PTR_ERR(regmap);
>>
>> - pll->l_reg = 0x04;
>> - pll->m_reg = 0x08;
>> - pll->n_reg = 0x0c;
>> - pll->config_reg = 0x14;
>> - pll->mode_reg = 0x00;
>> - pll->status_reg = 0x1c;
>> - pll->status_bit = 16;
>> - pll->freq_tbl = a53pll_freq;
>> -
>> - init.name = "a53pll";
>> - init.parent_names = (const char *[]){ "xo" };
>> - init.num_parents = 1;
>> - init.ops = &clk_pll_sr2_ops;
>> - init.flags = CLK_IS_CRITICAL;
> Please document why a clk is critical.
ok
>
>> - pll->clkr.hw.init = &init;
>> -
>> - ret = devm_clk_register_regmap(dev, &pll->clkr);
>> + if (pll_data->flags & PLL_IS_ALPHA) {
>> + struct clk_alpha_pll *alpha_pll =
>> + pll_data->a53pll.alpha_pll.pll;
>> + struct alpha_pll_config *alpha_pll_config =
>> + pll_data->a53pll.alpha_pll.pll_config;
>> +
>> + clk_alpha_pll_configure(alpha_pll, regmap, alpha_pll_config);
>> + clkr = &pll_data->a53pll.alpha_pll.pll->clkr;
>> + } else {
>> + clkr = &pll_data->a53pll.pll->clkr;
>> + }
> Sorry, the design is confusing.
The basic idea is to add support for various PLLs available to clock the
A53 core.
if this messing up the code, can the alpha pll support be moved to a
separate file?
It would be very helpful if you provide your input on this.
Hi Stephen,
On 4/22/2020 2:34 PM, Stephen Boyd wrote:
> Quoting Sivaprakash Murugesan (2020-04-13 19:55:19)
>> The CPU on Qualcomm's IPQ platform devices are clocked primarily by a
>> PLL and xo which are connected to a mux and enable block, This patch adds
> The comma should be a period? Don't write "This patch" in commit text.
ok.
>
>> support for the mux and the enable.
>>
>> Signed-off-by: Sivaprakash Murugesan <[email protected]>
>> ---
>> drivers/clk/qcom/Kconfig | 10 +++++
>> drivers/clk/qcom/Makefile | 1 +
>> drivers/clk/qcom/apss-ipq.c | 107 ++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 118 insertions(+)
>> create mode 100644 drivers/clk/qcom/apss-ipq.c
>>
>> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
>> index 15cdcdc..8573f2e 100644
>> --- a/drivers/clk/qcom/Kconfig
>> +++ b/drivers/clk/qcom/Kconfig
>> @@ -89,6 +89,16 @@ config APQ_MMCC_8084
>> Say Y if you want to support multimedia devices such as display,
>> graphics, video encode/decode, camera, etc.
>>
>> +config IPQ_APSS
>> + tristate "IPQ APSS Clock Controller"
>> + default N
> Drop this, it's already the default.
ok
>
>> + help
>> + Support for APSS clock controller on ipq platform devices. The
> Maybe say "IPQ platforms" and drop the devices part?
ok
>
>> + APSS clock controller manages the Mux and enable block that feeds the
>> + CPUs.
>> + Say Y if you want to support CPU frequency scaling on
>> + ipq based devices.
>> +
>> config IPQ_GCC_4019
>> tristate "IPQ4019 Global Clock Controller"
>> help
>> diff --git a/drivers/clk/qcom/apss-ipq.c b/drivers/clk/qcom/apss-ipq.c
>> new file mode 100644
>> index 0000000..a37cd98
>> --- /dev/null
>> +++ b/drivers/clk/qcom/apss-ipq.c
>> @@ -0,0 +1,107 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/err.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/regmap.h>
>> +#include <linux/module.h>
>> +
>> +#include <dt-bindings/clock/qcom,apss-ipq.h>
>> +
>> +#include "common.h"
>> +#include "clk-regmap.h"
>> +#include "clk-branch.h"
>> +#include "clk-alpha-pll.h"
>> +#include "clk-regmap-mux.h"
>> +
>> +enum {
>> + P_XO,
>> + P_APSS_PLL_EARLY,
>> +};
>> +
>> +static const struct clk_parent_data parents_apcs_alias0_clk_src[] = {
>> + { .fw_name = "xo" },
>> + { .fw_name = "pll" },
>> +};
>> +
>> +static const struct parent_map parents_apcs_alias0_clk_src_map[] = {
>> + { P_XO, 0 },
>> + { P_APSS_PLL_EARLY, 5 },
>> +};
>> +
>> +static struct clk_regmap_mux apcs_alias0_clk_src = {
>> + .reg = 0x0050,
>> + .width = 3,
>> + .shift = 7,
>> + .parent_map = parents_apcs_alias0_clk_src_map,
>> + .clkr.hw.init = &(struct clk_init_data){
>> + .name = "apcs_alias0_clk_src",
>> + .parent_data = parents_apcs_alias0_clk_src,
>> + .num_parents = 2,
>> + .ops = &clk_regmap_mux_closest_ops,
>> + .flags = CLK_SET_RATE_PARENT,
>> + },
>> +};
>> +
>> +/*required for cpufreq*/
> This comment doesn't help in understanding.
Just realized that it need not be critical. will remove it in next patch.
>
>> +static struct clk_branch apcs_alias0_core_clk = {
>> + .halt_reg = 0x0058,
>> + .clkr = {
>> + .enable_reg = 0x0058,
>> + .enable_mask = BIT(0),
>> + .hw.init = &(struct clk_init_data){
>> + .name = "apcs_alias0_core_clk",
>> + .parent_hws = (const struct clk_hw *[]){
>> + &apcs_alias0_clk_src.clkr.hw },
>> + .num_parents = 1,
>> + .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
> Please comment why the clk is critical.
>
>> + .ops = &clk_branch2_ops,
>> + },
>> + },
>> +};
>> +
>> +static const struct regmap_config apss_ipq_regmap_config = {
>> + .reg_bits = 32,
>> + .reg_stride = 4,
>> + .val_bits = 32,
>> + .max_register = 0x1000,
>> + .fast_io = true,
>> +};
>> +
>> +static struct clk_regmap *apss_ipq_clks[] = {
>> + [APCS_ALIAS0_CLK_SRC] = &apcs_alias0_clk_src.clkr,
>> + [APCS_ALIAS0_CORE_CLK] = &apcs_alias0_core_clk.clkr,
>> +};
>> +
>> +static const struct qcom_cc_desc apss_ipq_desc = {
>> + .config = &apss_ipq_regmap_config,
>> + .clks = apss_ipq_clks,
>> + .num_clks = ARRAY_SIZE(apss_ipq_clks),
>> +};
>> +
>> +static int apss_ipq_probe(struct platform_device *pdev)
>> +{
>> + struct regmap *regmap;
>> +
>> + regmap = dev_get_regmap(pdev->dev.parent, NULL);
>> + if (IS_ERR(regmap))
>> + return PTR_ERR(regmap);
>> +
>> + return qcom_cc_really_probe(pdev, &apss_ipq_desc, regmap);
> What is this a child of?
this is child of qcom apcs mailbox driver. will add compilation
dependency on next patch.
Quoting Sivaprakash Murugesan (2020-04-22 03:44:33)
> On 4/22/2020 2:30 PM, Stephen Boyd wrote:
> > Quoting Sivaprakash Murugesan (2020-04-13 19:55:17)
> >> diff --git a/drivers/clk/qcom/a53-pll.c b/drivers/clk/qcom/a53-pll.c
> >> index 45cfc57..a95351c 100644
> >> --- a/drivers/clk/qcom/a53-pll.c
> >> +++ b/drivers/clk/qcom/a53-pll.c
> >> @@ -57,30 +146,26 @@ static int qcom_a53pll_probe(struct platform_device *pdev)
> >> if (IS_ERR(regmap))
> >> return PTR_ERR(regmap);
> >>
> >> - pll->l_reg = 0x04;
> >> - pll->m_reg = 0x08;
> >> - pll->n_reg = 0x0c;
> >> - pll->config_reg = 0x14;
> >> - pll->mode_reg = 0x00;
> >> - pll->status_reg = 0x1c;
> >> - pll->status_bit = 16;
> >> - pll->freq_tbl = a53pll_freq;
> >> -
> >> - init.name = "a53pll";
> >> - init.parent_names = (const char *[]){ "xo" };
> >> - init.num_parents = 1;
> >> - init.ops = &clk_pll_sr2_ops;
> >> - init.flags = CLK_IS_CRITICAL;
> > Please document why a clk is critical.
> ok
> >
> >> - pll->clkr.hw.init = &init;
> >> -
> >> - ret = devm_clk_register_regmap(dev, &pll->clkr);
> >> + if (pll_data->flags & PLL_IS_ALPHA) {
> >> + struct clk_alpha_pll *alpha_pll =
> >> + pll_data->a53pll.alpha_pll.pll;
> >> + struct alpha_pll_config *alpha_pll_config =
> >> + pll_data->a53pll.alpha_pll.pll_config;
> >> +
> >> + clk_alpha_pll_configure(alpha_pll, regmap, alpha_pll_config);
> >> + clkr = &pll_data->a53pll.alpha_pll.pll->clkr;
> >> + } else {
> >> + clkr = &pll_data->a53pll.pll->clkr;
> >> + }
> > Sorry, the design is confusing.
>
> The basic idea is to add support for various PLLs available to clock the
> A53 core.
>
> if this messing up the code, can the alpha pll support be moved to a
> separate file?
>
> It would be very helpful if you provide your input on this.
Isn't the alpha PLL support already in a different file? Is it sometimes
an alpha pll and other times it is something else?
On 5/15/2020 2:10 AM, Stephen Boyd wrote:
> Quoting Sivaprakash Murugesan (2020-04-22 03:44:33)
>> On 4/22/2020 2:30 PM, Stephen Boyd wrote:
>>> Quoting Sivaprakash Murugesan (2020-04-13 19:55:17)
>>>> diff --git a/drivers/clk/qcom/a53-pll.c b/drivers/clk/qcom/a53-pll.c
>>>> index 45cfc57..a95351c 100644
>>>> --- a/drivers/clk/qcom/a53-pll.c
>>>> +++ b/drivers/clk/qcom/a53-pll.c
>>>> @@ -57,30 +146,26 @@ static int qcom_a53pll_probe(struct platform_device *pdev)
>>>> if (IS_ERR(regmap))
>>>> return PTR_ERR(regmap);
>>>>
>>>> - pll->l_reg = 0x04;
>>>> - pll->m_reg = 0x08;
>>>> - pll->n_reg = 0x0c;
>>>> - pll->config_reg = 0x14;
>>>> - pll->mode_reg = 0x00;
>>>> - pll->status_reg = 0x1c;
>>>> - pll->status_bit = 16;
>>>> - pll->freq_tbl = a53pll_freq;
>>>> -
>>>> - init.name = "a53pll";
>>>> - init.parent_names = (const char *[]){ "xo" };
>>>> - init.num_parents = 1;
>>>> - init.ops = &clk_pll_sr2_ops;
>>>> - init.flags = CLK_IS_CRITICAL;
>>> Please document why a clk is critical.
>> ok
>>>> - pll->clkr.hw.init = &init;
>>>> -
>>>> - ret = devm_clk_register_regmap(dev, &pll->clkr);
>>>> + if (pll_data->flags & PLL_IS_ALPHA) {
>>>> + struct clk_alpha_pll *alpha_pll =
>>>> + pll_data->a53pll.alpha_pll.pll;
>>>> + struct alpha_pll_config *alpha_pll_config =
>>>> + pll_data->a53pll.alpha_pll.pll_config;
>>>> +
>>>> + clk_alpha_pll_configure(alpha_pll, regmap, alpha_pll_config);
>>>> + clkr = &pll_data->a53pll.alpha_pll.pll->clkr;
>>>> + } else {
>>>> + clkr = &pll_data->a53pll.pll->clkr;
>>>> + }
>>> Sorry, the design is confusing.
>> The basic idea is to add support for various PLLs available to clock the
>> A53 core.
>>
>> if this messing up the code, can the alpha pll support be moved to a
>> separate file?
>>
>> It would be very helpful if you provide your input on this.
> Isn't the alpha PLL support already in a different file? Is it sometimes
> an alpha pll and other times it is something else?
alpha pll for cpufreq is not yet available, and for ipq based devices it
is alpha pll, and I guess
for other mobile based devices it is something else.
I have raised a patch set keeping the alpha pll as a separate file,
could you please take a
look into it?