[PATCH V6 0/5] Add APSS clock controller support for IPQ6018

[PATCH V6 0/5] Add APSS clock controller support for IPQ6018

[Sivaprakash Murugesan]

The CPU on Qualcomm's IPQ6018 devices are primarily fed by APSS 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.

[V6]
 * Split mailbox driver from this series, mailbox changes will sent as a
   separate series
 * Addressed review comments from Stephen
[V5]
 * Addressed Bjorn comments on apss clk and dt-bindings
 * Patch 2 depends on a53 pll dt-bindings
   https://www.spinics.net/lists/linux-clk/msg48358.html  
[V4]
 * Re-written PLL found on IPQ platforms as a separate driver
 * Addressed stephen's comments on apss clock controller and pll
 * Addressed Rob's review comments on bindings
 * moved a53 pll binding from this series as it is not applicable, will send
   it separately.
[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 (5):
  dt-bindings: clock: add ipq6018 a53 pll compatible
  clk: qcom: Add ipq apss pll driver
  clk: qcom: Add DT bindings for ipq6018 apss clock controller
  clk: qcom: Add ipq6018 apss clock controller
  arm64: dts: ipq6018: Add support for apss pll

 .../devicetree/bindings/clock/qcom,a53pll.yaml     |  18 ++++
 arch/arm64/boot/dts/qcom/ipq6018.dtsi              |   8 ++
 drivers/clk/qcom/Kconfig                           |  19 ++++
 drivers/clk/qcom/Makefile                          |   2 +
 drivers/clk/qcom/apss-ipq-pll.c                    |  95 ++++++++++++++++++
 drivers/clk/qcom/apss-ipq6018.c                    | 106 +++++++++++++++++++++
 include/dt-bindings/clock/qcom,apss-ipq.h          |  12 +++
 7 files changed, 260 insertions(+)
 create mode 100644 drivers/clk/qcom/apss-ipq-pll.c
 create mode 100644 drivers/clk/qcom/apss-ipq6018.c
 create mode 100644 include/dt-bindings/clock/qcom,apss-ipq.h

-- 
2.7.4

[PATCH V6 1/5] dt-bindings: clock: add ipq6018 a53 pll compatible

[Sivaprakash Murugesan]

cpus on ipq6018 are clocked by a53 pll, add device compatible for a53
pll found on ipq6018 devices.

Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
---
* [V6]
    re-ordered compatible string, dropped Rob's review tag for this change.
 .../devicetree/bindings/clock/qcom,a53pll.yaml         | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml b/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
index 20d2638..a4f2d01 100644
--- a/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
@@ -15,6 +15,7 @@ description:
 
 properties:
   compatible:
+    const: qcom,ipq6018-a53pll
     const: qcom,msm8916-a53pll
 
   reg:
@@ -23,6 +24,14 @@ properties:
   '#clock-cells':
     const: 0
 
+  clocks:
+    items:
+      - description: board XO clock
+
+  clock-names:
+    items:
+      - const: xo
+
 required:
   - compatible
   - reg
@@ -38,3 +47,12 @@ examples:
         reg = <0xb016000 0x40>;
         #clock-cells = <0>;
     };
+  #Example 2 - A53 PLL found on IPQ6018 devices
+  - |
+    a53pll_ipq: clock@b116000 {
+        compatible = "qcom,ipq6018-a53pll";
+        reg = <0x0b116000 0x40>;
+        #clock-cells = <0>;
+        clocks = <&xo>;
+        clock-names = "xo";
+    };
-- 
2.7.4

Re: [PATCH V6 1/5] dt-bindings: clock: add ipq6018 a53 pll compatible

[Rob Herring]

On Wed, May 27, 2020 at 05:54:48PM +0530, Sivaprakash Murugesan wrote:
> cpus on ipq6018 are clocked by a53 pll, add device compatible for a53
> pll found on ipq6018 devices.
> 
> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
> ---
> * [V6]
>     re-ordered compatible string, dropped Rob's review tag for this change.

Not really significant enough to drop it, but if you really want me to 
stare at this again...

>  .../devicetree/bindings/clock/qcom,a53pll.yaml         | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml b/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
> index 20d2638..a4f2d01 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
> @@ -15,6 +15,7 @@ description:
>  
>  properties:
>    compatible:
> +    const: qcom,ipq6018-a53pll
>      const: qcom,msm8916-a53pll
>  
>    reg:
> @@ -23,6 +24,14 @@ properties:
>    '#clock-cells':
>      const: 0
>  
> +  clocks:
> +    items:
> +      - description: board XO clock
> +
> +  clock-names:
> +    items:
> +      - const: xo
> +
>  required:
>    - compatible
>    - reg
> @@ -38,3 +47,12 @@ examples:
>          reg = <0xb016000 0x40>;
>          #clock-cells = <0>;
>      };
> +  #Example 2 - A53 PLL found on IPQ6018 devices
> +  - |
> +    a53pll_ipq: clock@b116000 {

clock-controller@...

> +        compatible = "qcom,ipq6018-a53pll";
> +        reg = <0x0b116000 0x40>;
> +        #clock-cells = <0>;
> +        clocks = <&xo>;
> +        clock-names = "xo";
> +    };
> -- 
> 2.7.4
> 

[PATCH V6 2/5] clk: qcom: Add ipq apss pll driver

[Sivaprakash Murugesan]

The CPUs on Qualcomm ipq based devices are clocked by an alpha PLL.
Add support for the apss pll found on ipq based devices which can
support CPU frequencies above 1Ghz.

Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
---
[V6]
 * Addressed review comments from Stephen
 drivers/clk/qcom/Kconfig        |  8 ++++
 drivers/clk/qcom/Makefile       |  1 +
 drivers/clk/qcom/apss-ipq-pll.c | 95 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 104 insertions(+)
 create mode 100644 drivers/clk/qcom/apss-ipq-pll.c

diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index 11ec6f4..e70aa01 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -89,6 +89,14 @@ 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_PLL
+	tristate "IPQ APSS PLL"
+	help
+	  Support for APSS PLL on ipq devices. The APSS PLL is the main
+	  clock that feeds the CPUs on ipq based devices.
+	  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 691efbf..b4a6ba1 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_PLL) += apss-ipq-pll.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-pll.c b/drivers/clk/qcom/apss-ipq-pll.c
new file mode 100644
index 0000000..e34f4cd
--- /dev/null
+++ b/drivers/clk/qcom/apss-ipq-pll.c
@@ -0,0 +1,95 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018, The Linux Foundation. All rights reserved.
+#include <linux/clk-provider.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#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 struct clk_alpha_pll ipq_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",
+			.parent_data = &(const struct clk_parent_data) {
+				.fw_name = "xo",
+			},
+			.num_parents = 1,
+			.ops = &clk_alpha_pll_huayra_ops,
+		},
+	},
+};
+
+static const struct alpha_pll_config ipq_pll_config = {
+	.l = 0x37,
+	.config_ctl_val = 0x04141200,
+	.config_ctl_hi_val = 0x0,
+	.early_output_mask = BIT(3),
+	.main_output_mask = BIT(0),
+};
+
+static const struct regmap_config ipq_pll_regmap_config = {
+	.reg_bits		= 32,
+	.reg_stride		= 4,
+	.val_bits		= 32,
+	.max_register		= 0x40,
+	.fast_io		= true,
+};
+
+static int apss_ipq_pll_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct regmap *regmap;
+	void __iomem *base;
+	int ret;
+
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	regmap = devm_regmap_init_mmio(dev, base, &ipq_pll_regmap_config);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	clk_alpha_pll_configure(&ipq_pll, regmap, &ipq_pll_config);
+
+	ret = devm_clk_register_regmap(dev, &ipq_pll.clkr);
+	if (ret)
+		return ret;
+
+	return devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get,
+							&ipq_pll.clkr.hw);
+}
+
+static const struct of_device_id apss_ipq_pll_match_table[] = {
+	{ .compatible = "qcom,ipq6018-a53pll" },
+	{ }
+};
+
+static struct platform_driver apss_ipq_pll_driver = {
+	.probe = apss_ipq_pll_probe,
+	.driver = {
+		.name = "qcom-ipq-apss-pll",
+		.of_match_table = apss_ipq_pll_match_table,
+	},
+};
+module_platform_driver(apss_ipq_pll_driver);
+
+MODULE_DESCRIPTION("Qualcomm technology Inc APSS ALPHA PLL Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

[PATCH V6 3/5] clk: qcom: Add DT bindings for ipq6018 apss clock controller

[Sivaprakash Murugesan]

Add dt-binding for ipq6018 apss clock controller

Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
---
[V6]
 * Addressed review comment from Stephen
 include/dt-bindings/clock/qcom,apss-ipq.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)
 create mode 100644 include/dt-bindings/clock/qcom,apss-ipq.h

diff --git a/include/dt-bindings/clock/qcom,apss-ipq.h b/include/dt-bindings/clock/qcom,apss-ipq.h
new file mode 100644
index 0000000..77b6e05
--- /dev/null
+++ b/include/dt-bindings/clock/qcom,apss-ipq.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+#ifndef _DT_BINDINGS_CLOCK_QCA_APSS_IPQ6018_H
+#define _DT_BINDINGS_CLOCK_QCA_APSS_IPQ6018_H
+
+#define APCS_ALIAS0_CLK_SRC			0
+#define APCS_ALIAS0_CORE_CLK			1
+
+#endif
-- 
2.7.4

Re: [PATCH V6 3/5] clk: qcom: Add DT bindings for ipq6018 apss clock controller

[Rob Herring]

On Wed, 27 May 2020 17:54:50 +0530, Sivaprakash Murugesan wrote:
> Add dt-binding for ipq6018 apss clock controller
> 
> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
> ---
> [V6]
>  * Addressed review comment from Stephen
>  include/dt-bindings/clock/qcom,apss-ipq.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>  create mode 100644 include/dt-bindings/clock/qcom,apss-ipq.h
> 

Acked-by: Rob Herring <robh@kernel.org>

[PATCH V6 4/5] clk: qcom: Add ipq6018 apss clock controller

[Sivaprakash Murugesan]

The CPU on Qualcomm ipq6018 devices are clocked primarily by a aplha PLL
and xo which are connected to a mux and enable block.

Add support for the mux and enable block which feeds the CPU on ipq6018
devices.

Reviewed-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
---
 drivers/clk/qcom/Kconfig        |  11 +++++
 drivers/clk/qcom/Makefile       |   1 +
 drivers/clk/qcom/apss-ipq6018.c | 106 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 118 insertions(+)
 create mode 100644 drivers/clk/qcom/apss-ipq6018.c

diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index e70aa01..b543e63 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -97,6 +97,17 @@ config IPQ_APSS_PLL
 	  Say Y if you want to support CPU frequency scaling on ipq based
 	  devices.
 
+config IPQ_APSS_6018
+	tristate "IPQ APSS Clock Controller"
+	select IPQ_APSS_PLL
+	depends on QCOM_APCS_IPC || COMPILE_TEST
+	help
+	  Support for APSS clock controller on IPQ platforms. 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 b4a6ba1..3accea1 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -20,6 +20,7 @@ clk-qcom-$(CONFIG_QCOM_GDSC) += gdsc.o
 obj-$(CONFIG_APQ_GCC_8084) += gcc-apq8084.o
 obj-$(CONFIG_APQ_MMCC_8084) += mmcc-apq8084.o
 obj-$(CONFIG_IPQ_APSS_PLL) += apss-ipq-pll.o
+obj-$(CONFIG_IPQ_APSS_6018) += apss-ipq6018.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-ipq6018.c b/drivers/clk/qcom/apss-ipq6018.c
new file mode 100644
index 0000000..004f7e1
--- /dev/null
+++ b/drivers/clk/qcom/apss-ipq6018.c
@@ -0,0 +1,106 @@
+// 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,
+	},
+};
+
+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,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static const struct regmap_config apss_ipq6018_regmap_config = {
+	.reg_bits       = 32,
+	.reg_stride     = 4,
+	.val_bits       = 32,
+	.max_register   = 0x1000,
+	.fast_io        = true,
+};
+
+static struct clk_regmap *apss_ipq6018_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_ipq6018_desc = {
+	.config = &apss_ipq6018_regmap_config,
+	.clks = apss_ipq6018_clks,
+	.num_clks = ARRAY_SIZE(apss_ipq6018_clks),
+};
+
+static int apss_ipq6018_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_ipq6018_desc, regmap);
+}
+
+static struct platform_driver apss_ipq6018_driver = {
+	.probe = apss_ipq6018_probe,
+	.driver = {
+		.name   = "qcom,apss-ipq6018-clk",
+	},
+};
+
+module_platform_driver(apss_ipq6018_driver);
+
+MODULE_DESCRIPTION("QCOM APSS IPQ 6018 CLK Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

Re: [PATCH V6 4/5] clk: qcom: Add ipq6018 apss clock controller

[Stephen Boyd]

Quoting Sivaprakash Murugesan (2020-05-27 05:24:51)
> diff --git a/drivers/clk/qcom/apss-ipq6018.c b/drivers/clk/qcom/apss-ipq6018.c
> new file mode 100644
> index 0000000..004f7e1
> --- /dev/null
> +++ b/drivers/clk/qcom/apss-ipq6018.c
> @@ -0,0 +1,106 @@
> +// 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" },

This pll clk is not described in the binding. Please add it there.

> +};
> +
> +static const struct parent_map parents_apcs_alias0_clk_src_map[] = {
> +       { P_XO, 0 },
> +       { P_APSS_PLL_EARLY, 5 },
> +};
> +

Re: [PATCH V6 4/5] clk: qcom: Add ipq6018 apss clock controller

[Sivaprakash Murugesan]

Hi Stepen,

On 5/28/2020 7:29 AM, Stephen Boyd wrote:
> Quoting Sivaprakash Murugesan (2020-05-27 05:24:51)
>> diff --git a/drivers/clk/qcom/apss-ipq6018.c b/drivers/clk/qcom/apss-ipq6018.c
>> new file mode 100644
>> index 0000000..004f7e1
>> --- /dev/null
>> +++ b/drivers/clk/qcom/apss-ipq6018.c
>> @@ -0,0 +1,106 @@
>> +// 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" },
> This pll clk is not described in the binding. Please add it there.

Sorry I did not get this, this PLL is not directly defined in this 
driver and it comes

from dts. do you still want to describe it in binding?

Re: [PATCH V6 4/5] clk: qcom: Add ipq6018 apss clock controller

[Stephen Boyd]

Quoting Sivaprakash Murugesan (2020-06-01 05:41:15)
> On 5/28/2020 7:29 AM, Stephen Boyd wrote:
> > Quoting Sivaprakash Murugesan (2020-05-27 05:24:51)
> >> diff --git a/drivers/clk/qcom/apss-ipq6018.c b/drivers/clk/qcom/apss-ipq6018.c
> >> new file mode 100644
> >> index 0000000..004f7e1
> >> --- /dev/null
> >> +++ b/drivers/clk/qcom/apss-ipq6018.c
> >> @@ -0,0 +1,106 @@
> >> +       P_XO,
> >> +       P_APSS_PLL_EARLY,
> >> +};
> >> +
> >> +static const struct clk_parent_data parents_apcs_alias0_clk_src[] = {
> >> +       { .fw_name = "xo" },
> >> +       { .fw_name = "pll" },
> > This pll clk is not described in the binding. Please add it there.
> 
> Sorry I did not get this, this PLL is not directly defined in this 
> driver and it comes
> 
> from dts. do you still want to describe it in binding?
> 

Yes, there should be a clock-names property for "pll" and a clocks
property in the binding document. I didn't see that.

Re: [PATCH V6 4/5] clk: qcom: Add ipq6018 apss clock controller

[Sivaprakash Murugesan]

On 6/2/2020 1:06 AM, Stephen Boyd wrote:
> Quoting Sivaprakash Murugesan (2020-06-01 05:41:15)
>> On 5/28/2020 7:29 AM, Stephen Boyd wrote:
>>> Quoting Sivaprakash Murugesan (2020-05-27 05:24:51)
>>>> diff --git a/drivers/clk/qcom/apss-ipq6018.c b/drivers/clk/qcom/apss-ipq6018.c
>>>> new file mode 100644
>>>> index 0000000..004f7e1
>>>> --- /dev/null
>>>> +++ b/drivers/clk/qcom/apss-ipq6018.c
>>>> @@ -0,0 +1,106 @@
>>>> +       P_XO,
>>>> +       P_APSS_PLL_EARLY,
>>>> +};
>>>> +
>>>> +static const struct clk_parent_data parents_apcs_alias0_clk_src[] = {
>>>> +       { .fw_name = "xo" },
>>>> +       { .fw_name = "pll" },
>>> This pll clk is not described in the binding. Please add it there.
>> Sorry I did not get this, this PLL is not directly defined in this
>> driver and it comes
>>
>> from dts. do you still want to describe it in binding?
>>
> Yes, there should be a clock-names property for "pll" and a clocks
> property in the binding document. I didn't see that.

These are defined in

https://lkml.org/lkml/2020/5/27/658and

https://lkml.org/lkml/2020/5/27/659

it has been defined as part of mailbox binding, since this driver does

not have a dts node and it is child of apcs mailbox driver.

Re: [PATCH V6 4/5] clk: qcom: Add ipq6018 apss clock controller

[Stephen Boyd]

Quoting Sivaprakash Murugesan (2020-06-02 03:47:20)
> 
> On 6/2/2020 1:06 AM, Stephen Boyd wrote:
> > Quoting Sivaprakash Murugesan (2020-06-01 05:41:15)
> >> On 5/28/2020 7:29 AM, Stephen Boyd wrote:
> >>> Quoting Sivaprakash Murugesan (2020-05-27 05:24:51)
> >>>> diff --git a/drivers/clk/qcom/apss-ipq6018.c b/drivers/clk/qcom/apss-ipq6018.c
> >>>> new file mode 100644
> >>>> index 0000000..004f7e1
> >>>> --- /dev/null
> >>>> +++ b/drivers/clk/qcom/apss-ipq6018.c
> >>>> @@ -0,0 +1,106 @@
> >>>> +       P_XO,
> >>>> +       P_APSS_PLL_EARLY,
> >>>> +};
> >>>> +
> >>>> +static const struct clk_parent_data parents_apcs_alias0_clk_src[] = {
> >>>> +       { .fw_name = "xo" },
> >>>> +       { .fw_name = "pll" },
> >>> This pll clk is not described in the binding. Please add it there.
> >> Sorry I did not get this, this PLL is not directly defined in this
> >> driver and it comes
> >>
> >> from dts. do you still want to describe it in binding?
> >>
> > Yes, there should be a clock-names property for "pll" and a clocks
> > property in the binding document. I didn't see that.
> 
> These are defined in
> 
> https://lkml.org/lkml/2020/5/27/658and
> 
> https://lkml.org/lkml/2020/5/27/659
> 
> it has been defined as part of mailbox binding, since this driver does
> 
> not have a dts node and it is child of apcs mailbox driver.
> 

Ah alright. Sounds good.

[PATCH V6 5/5] arm64: dts: ipq6018: Add support for apss pll

[Sivaprakash Murugesan]

Enable apss pll support.

Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
---
[V6]
 * split the mailbox driver from this patch
 arch/arm64/boot/dts/qcom/ipq6018.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
index 1aa8d85..3956e44 100644
--- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
@@ -300,6 +300,14 @@
 			#mbox-cells = <1>;
 		};
 
+		apsspll: 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

Re: [PATCH V6 5/5] arm64: dts: ipq6018: Add support for apss pll

[Stephen Boyd]

Quoting Sivaprakash Murugesan (2020-05-27 05:24:52)
> Enable apss pll support.
> 
> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
> ---
> [V6]
>  * split the mailbox driver from this patch
>  arch/arm64/boot/dts/qcom/ipq6018.dtsi | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> index 1aa8d85..3956e44 100644
> --- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> @@ -300,6 +300,14 @@
>                         #mbox-cells = <1>;
>                 };
>  
> +               apsspll: clock@b116000 {
> +                       compatible = "qcom,ipq6018-a53pll";
> +                       reg = <0x0b116000 0x40>;
> +                       #clock-cells = <0>;
> +                       clocks = <&xo>;
> +                       clock-names = "xo";
> +               };
> +

I'd expect to see this inside an soc node. Also this doesn't go via clk
tree so don't send it with the clk patches.

>                 timer {
>                         compatible = "arm,armv8-timer";
>                         interrupts = <GIC_PPI 2 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,