linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V3 0/8] Add APSS clock controller support for IPQ6018
@ 2020-04-14  2:55 Sivaprakash Murugesan
  2020-04-14  2:55 ` [PATCH V3 1/8] dt-bindings: mailbox: Add YAML schemas for QCOM APCS global block Sivaprakash Murugesan
                   ` (7 more replies)
  0 siblings, 8 replies; 21+ messages in thread
From: Sivaprakash Murugesan @ 2020-04-14  2:55 UTC (permalink / raw)
  To: agross, bjorn.andersson, mturquette, sboyd, robh+dt,
	jassisinghbrar, linux-arm-msm, linux-clk, devicetree,
	linux-kernel
  Cc: Sivaprakash Murugesan

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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH V3 1/8] dt-bindings: mailbox: Add YAML schemas for QCOM APCS global block
  2020-04-14  2:55 [PATCH V3 0/8] Add APSS clock controller support for IPQ6018 Sivaprakash Murugesan
@ 2020-04-14  2:55 ` Sivaprakash Murugesan
  2020-04-20 20:59   ` Rob Herring
  2020-04-14  2:55 ` [PATCH V3 2/8] dt-bindings: clock: Add YAML schemas for QCOM A53 PLL Sivaprakash Murugesan
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: Sivaprakash Murugesan @ 2020-04-14  2:55 UTC (permalink / raw)
  To: agross, bjorn.andersson, mturquette, sboyd, robh+dt,
	jassisinghbrar, linux-arm-msm, linux-clk, devicetree,
	linux-kernel
  Cc: Sivaprakash Murugesan

Qualcomm APCS global block provides a bunch of generic properties which
are required in a device tree. Add YAML schema for these properties.

Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
---
 .../bindings/mailbox/qcom,apcs-kpss-global.txt     | 88 ----------------------
 .../bindings/mailbox/qcom,apcs-kpss-global.yaml    | 88 ++++++++++++++++++++++
 2 files changed, 88 insertions(+), 88 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt
 create mode 100644 Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml

diff --git a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt
deleted file mode 100644
index beec612..0000000
--- a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt
+++ /dev/null
@@ -1,88 +0,0 @@
-Binding for the Qualcomm APCS global block
-==========================================
-
-This binding describes the APCS "global" block found in various Qualcomm
-platforms.
-
-- compatible:
-	Usage: required
-	Value type: <string>
-	Definition: must be one of:
-		    "qcom,msm8916-apcs-kpss-global",
-		    "qcom,msm8996-apcs-hmss-global"
-		    "qcom,msm8998-apcs-hmss-global"
-		    "qcom,qcs404-apcs-apps-global"
-		    "qcom,sc7180-apss-shared"
-		    "qcom,sdm845-apss-shared"
-		    "qcom,sm8150-apss-shared"
-		    "qcom,ipq8074-apcs-apps-global"
-
-- reg:
-	Usage: required
-	Value type: <prop-encoded-array>
-	Definition: must specify the base address and size of the global block
-
-- clocks:
-	Usage: required if #clock-names property is present
-	Value type: <phandle array>
-	Definition: phandles to the two parent clocks of the clock driver.
-
-- #mbox-cells:
-	Usage: required
-	Value type: <u32>
-	Definition: as described in mailbox.txt, must be 1
-
-- #clock-cells:
-	Usage: optional
-	Value type: <u32>
-	Definition: as described in clock.txt, must be 0
-
-- clock-names:
-	Usage: required if the platform data based clock driver needs to
-	retrieve the parent clock names from device tree.
-	This will requires two mandatory clocks to be defined.
-	Value type: <string-array>
-	Definition: must be "pll" and "aux"
-
-= EXAMPLE
-The following example describes the APCS HMSS found in MSM8996 and part of the
-GLINK RPM referencing the "rpm_hlos" doorbell therein.
-
-	apcs_glb: mailbox@9820000 {
-		compatible = "qcom,msm8996-apcs-hmss-global";
-		reg = <0x9820000 0x1000>;
-
-		#mbox-cells = <1>;
-	};
-
-	rpm-glink {
-		compatible = "qcom,glink-rpm";
-
-		interrupts = <GIC_SPI 168 IRQ_TYPE_EDGE_RISING>;
-
-		qcom,rpm-msg-ram = <&rpm_msg_ram>;
-
-		mboxes = <&apcs_glb 0>;
-		mbox-names = "rpm_hlos";
-	};
-
-Below is another example of the APCS binding on MSM8916 platforms:
-
-	apcs: mailbox@b011000 {
-		compatible = "qcom,msm8916-apcs-kpss-global";
-		reg = <0xb011000 0x1000>;
-		#mbox-cells = <1>;
-		clocks = <&a53pll>;
-		#clock-cells = <0>;
-	};
-
-Below is another example of the APCS binding on QCS404 platforms:
-
-	apcs_glb: mailbox@b011000 {
-		compatible = "qcom,qcs404-apcs-apps-global", "syscon";
-		reg = <0x0b011000 0x1000>;
-		#mbox-cells = <1>;
-		clocks = <&apcs_hfpll>, <&gcc GCC_GPLL0_AO_OUT_MAIN>;
-		clock-names = "pll", "aux";
-		#clock-cells = <0>;
-	};
diff --git a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
new file mode 100644
index 0000000..b46474b
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
@@ -0,0 +1,88 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/mailbox/qcom,apcs-kpss-global.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Qualcomm APCS global block bindings
+
+description:
+  This binding describes the APCS "global" block found in various Qualcomm
+  platforms.
+
+maintainers:
+  - Sivaprakash Murugesan <sivaprak@codeaurora.org>
+
+properties:
+  compatible:
+    enum:
+      - qcom,ipq8074-apcs-apps-global
+      - qcom,msm8916-apcs-kpss-global
+      - qcom,msm8996-apcs-hmss-global
+      - qcom,msm8998-apcs-hmss-global
+      - qcom,qcs404-apcs-apps-global
+      - qcom,sc7180-apss-shared
+      - qcom,sdm845-apss-shared
+      - qcom,sm8150-apss-shared
+
+  reg:
+    description: specifies the base address and size of the global block
+    maxItems: 1
+
+  clocks:
+    description: phandles to the parent clocks of the clock driver
+
+  '#mbox-cells':
+    const: 1
+
+  '#clock-cells':
+    const: 0
+
+  clock-names:
+    description:
+      parent clock names, required if the platform data based clock driver
+      needs to retrieve the parent clock names from device tree.
+    maxItems: 2
+    items:
+      - const: pll
+      - const: aux
+
+required:
+  - compatible
+  - reg
+  - '#mbox-cells'
+
+additionalProperties: false
+
+examples:
+
+  # Example apcs with msm8996
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    apcs_glb: mailbox@9820000 {
+        compatible = "qcom,msm8996-apcs-hmss-global";
+        reg = <0x9820000 0x1000>;
+
+        #mbox-cells = <1>;
+    };
+
+    rpm-glink {
+        compatible = "qcom,glink-rpm";
+        interrupts = <GIC_SPI 168 IRQ_TYPE_EDGE_RISING>;
+        qcom,rpm-msg-ram = <&rpm_msg_ram>;
+        mboxes = <&apcs_glb 0>;
+        mbox-names = "rpm_hlos";
+    };
+
+  # Example apcs with qcs404
+  - |
+    #define GCC_APSS_AHB_CLK_SRC  1
+    #define GCC_GPLL0_AO_OUT_MAIN 123
+    apcs: mailbox@b011000 {
+        compatible = "qcom,qcs404-apcs-apps-global";
+        reg = <0x0b011000 0x1000>;
+        #mbox-cells = <1>;
+        clocks = <&apcs_hfpll>, <&gcc GCC_GPLL0_AO_OUT_MAIN>;
+        clock-names = "pll", "aux";
+        #clock-cells = <0>;
+    };
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH V3 2/8] dt-bindings: clock: Add YAML schemas for QCOM A53 PLL
  2020-04-14  2:55 [PATCH V3 0/8] Add APSS clock controller support for IPQ6018 Sivaprakash Murugesan
  2020-04-14  2:55 ` [PATCH V3 1/8] dt-bindings: mailbox: Add YAML schemas for QCOM APCS global block Sivaprakash Murugesan
@ 2020-04-14  2:55 ` Sivaprakash Murugesan
  2020-04-20 21:01   ` Rob Herring
  2020-04-20 21:03   ` Rob Herring
  2020-04-14  2:55 ` [PATCH V3 3/8] clk: qcom: Add A53 PLL support for ipq6018 devices Sivaprakash Murugesan
                   ` (5 subsequent siblings)
  7 siblings, 2 replies; 21+ messages in thread
From: Sivaprakash Murugesan @ 2020-04-14  2:55 UTC (permalink / raw)
  To: agross, bjorn.andersson, mturquette, sboyd, robh+dt,
	jassisinghbrar, linux-arm-msm, linux-clk, devicetree,
	linux-kernel
  Cc: Sivaprakash Murugesan

This patch adds schema for primary CPU PLL found on few Qualcomm
platforms.

Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
---
[V3]
 * Fixed dt binding error in "$id" field.

 .../devicetree/bindings/clock/qcom,a53pll.txt      | 22 --------
 .../devicetree/bindings/clock/qcom,a53pll.yaml     | 60 ++++++++++++++++++++++
 2 files changed, 60 insertions(+), 22 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/clock/qcom,a53pll.txt
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,a53pll.yaml

diff --git a/Documentation/devicetree/bindings/clock/qcom,a53pll.txt b/Documentation/devicetree/bindings/clock/qcom,a53pll.txt
deleted file mode 100644
index e3fa811..0000000
--- a/Documentation/devicetree/bindings/clock/qcom,a53pll.txt
+++ /dev/null
@@ -1,22 +0,0 @@
-Qualcomm MSM8916 A53 PLL Binding
---------------------------------
-The A53 PLL on MSM8916 platforms is the main CPU PLL used used for frequencies
-above 1GHz.
-
-Required properties :
-- compatible : Shall contain only one of the following:
-
-		"qcom,msm8916-a53pll"
-
-- reg : shall contain base register location and length
-
-- #clock-cells : must be set to <0>
-
-Example:
-
-	a53pll: clock@b016000 {
-		compatible = "qcom,msm8916-a53pll";
-		reg = <0xb016000 0x40>;
-		#clock-cells = <0>;
-	};
-
diff --git a/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml b/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
new file mode 100644
index 0000000..c865293
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/qcom,a53pll.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm A53 PLL Binding
+
+maintainers:
+  - Sivaprakash Murugesan <sivaprak@codeaurora.org>
+
+description:
+  The A53 PLL on few Qualcomm platforms is the main CPU PLL used used for
+  frequencies above 1GHz.
+
+properties:
+  compatible:
+    enum:
+      - qcom,msm8916-a53pll
+      - qcom,ipq6018-a53pll
+
+  reg:
+    maxItems: 1
+
+  '#clock-cells':
+    const: 0
+
+  clocks:
+    description: clocks required for this controller.
+    maxItems: 1
+
+  clock-names:
+    description: clock output names of required clocks.
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - '#clock-cells'
+
+additionalProperties: false
+
+examples:
+  #Example 1 - A53 PLL found on MSM8916 devices
+  - |
+    a53pll: clock@b016000 {
+        compatible = "qcom,msm8916-a53pll";
+        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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH V3 3/8] clk: qcom: Add A53 PLL support for ipq6018 devices
  2020-04-14  2:55 [PATCH V3 0/8] Add APSS clock controller support for IPQ6018 Sivaprakash Murugesan
  2020-04-14  2:55 ` [PATCH V3 1/8] dt-bindings: mailbox: Add YAML schemas for QCOM APCS global block Sivaprakash Murugesan
  2020-04-14  2:55 ` [PATCH V3 2/8] dt-bindings: clock: Add YAML schemas for QCOM A53 PLL Sivaprakash Murugesan
@ 2020-04-14  2:55 ` Sivaprakash Murugesan
  2020-04-22  9:00   ` Stephen Boyd
  2020-04-14  2:55 ` [PATCH V3 4/8] clk: qcom: Add DT bindings for ipq6018 apss clock controller Sivaprakash Murugesan
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: Sivaprakash Murugesan @ 2020-04-14  2:55 UTC (permalink / raw)
  To: agross, bjorn.andersson, mturquette, sboyd, robh+dt,
	jassisinghbrar, linux-arm-msm, linux-clk, devicetree,
	linux-kernel
  Cc: Sivaprakash Murugesan

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 <sivaprak@codeaurora.org>
---
 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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH V3 4/8] clk: qcom: Add DT bindings for ipq6018 apss clock controller
  2020-04-14  2:55 [PATCH V3 0/8] Add APSS clock controller support for IPQ6018 Sivaprakash Murugesan
                   ` (2 preceding siblings ...)
  2020-04-14  2:55 ` [PATCH V3 3/8] clk: qcom: Add A53 PLL support for ipq6018 devices Sivaprakash Murugesan
@ 2020-04-14  2:55 ` Sivaprakash Murugesan
  2020-04-14  2:55 ` [PATCH V3 5/8] clk: qcom: Add ipq " Sivaprakash Murugesan
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 21+ messages in thread
From: Sivaprakash Murugesan @ 2020-04-14  2:55 UTC (permalink / raw)
  To: agross, bjorn.andersson, mturquette, sboyd, robh+dt,
	jassisinghbrar, linux-arm-msm, linux-clk, devicetree,
	linux-kernel
  Cc: Sivaprakash Murugesan

add dt-binding for ipq6018 apss clock controller

Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
---
 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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH V3 5/8] clk: qcom: Add ipq apss clock controller
  2020-04-14  2:55 [PATCH V3 0/8] Add APSS clock controller support for IPQ6018 Sivaprakash Murugesan
                   ` (3 preceding siblings ...)
  2020-04-14  2:55 ` [PATCH V3 4/8] clk: qcom: Add DT bindings for ipq6018 apss clock controller Sivaprakash Murugesan
@ 2020-04-14  2:55 ` Sivaprakash Murugesan
  2020-04-22  9:04   ` Stephen Boyd
  2020-04-14  2:55 ` [PATCH V3 6/8] dt-bindings: mailbox: Add dt-bindings for ipq6018 apcs global block Sivaprakash Murugesan
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: Sivaprakash Murugesan @ 2020-04-14  2:55 UTC (permalink / raw)
  To: agross, bjorn.andersson, mturquette, sboyd, robh+dt,
	jassisinghbrar, linux-arm-msm, linux-clk, devicetree,
	linux-kernel
  Cc: Sivaprakash Murugesan

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 <sivaprak@codeaurora.org>
---
 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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH V3 6/8] dt-bindings: mailbox: Add dt-bindings for ipq6018 apcs global block
  2020-04-14  2:55 [PATCH V3 0/8] Add APSS clock controller support for IPQ6018 Sivaprakash Murugesan
                   ` (4 preceding siblings ...)
  2020-04-14  2:55 ` [PATCH V3 5/8] clk: qcom: Add ipq " Sivaprakash Murugesan
@ 2020-04-14  2:55 ` Sivaprakash Murugesan
  2020-04-20 21:05   ` Rob Herring
  2020-04-14  2:55 ` [PATCH V3 7/8] mailbox: qcom: Add ipq6018 apcs compatible Sivaprakash Murugesan
  2020-04-14  2:55 ` [PATCH V3 8/8] arm64: dts: ipq6018: Add a53 pll and apcs clock Sivaprakash Murugesan
  7 siblings, 1 reply; 21+ messages in thread
From: Sivaprakash Murugesan @ 2020-04-14  2:55 UTC (permalink / raw)
  To: agross, bjorn.andersson, mturquette, sboyd, robh+dt,
	jassisinghbrar, linux-arm-msm, linux-clk, devicetree,
	linux-kernel
  Cc: Sivaprakash Murugesan

Add dt-bindings for ipq6018 mailbox driver

Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
---
 .../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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH V3 7/8] mailbox: qcom: Add ipq6018 apcs compatible
  2020-04-14  2:55 [PATCH V3 0/8] Add APSS clock controller support for IPQ6018 Sivaprakash Murugesan
                   ` (5 preceding siblings ...)
  2020-04-14  2:55 ` [PATCH V3 6/8] dt-bindings: mailbox: Add dt-bindings for ipq6018 apcs global block Sivaprakash Murugesan
@ 2020-04-14  2:55 ` Sivaprakash Murugesan
  2020-04-14  2:55 ` [PATCH V3 8/8] arm64: dts: ipq6018: Add a53 pll and apcs clock Sivaprakash Murugesan
  7 siblings, 0 replies; 21+ messages in thread
From: Sivaprakash Murugesan @ 2020-04-14  2:55 UTC (permalink / raw)
  To: agross, bjorn.andersson, mturquette, sboyd, robh+dt,
	jassisinghbrar, linux-arm-msm, linux-clk, devicetree,
	linux-kernel
  Cc: Sivaprakash Murugesan

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 <sivaprak@codeaurora.org>
---
 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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH V3 8/8] arm64: dts: ipq6018: Add a53 pll and apcs clock
  2020-04-14  2:55 [PATCH V3 0/8] Add APSS clock controller support for IPQ6018 Sivaprakash Murugesan
                   ` (6 preceding siblings ...)
  2020-04-14  2:55 ` [PATCH V3 7/8] mailbox: qcom: Add ipq6018 apcs compatible Sivaprakash Murugesan
@ 2020-04-14  2:55 ` Sivaprakash Murugesan
  7 siblings, 0 replies; 21+ messages in thread
From: Sivaprakash Murugesan @ 2020-04-14  2:55 UTC (permalink / raw)
  To: agross, bjorn.andersson, mturquette, sboyd, robh+dt,
	jassisinghbrar, linux-arm-msm, linux-clk, devicetree,
	linux-kernel
  Cc: Sivaprakash Murugesan

add support for a53 pll and apcs clock.

Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
---
 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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH V3 1/8] dt-bindings: mailbox: Add YAML schemas for QCOM APCS global block
  2020-04-14  2:55 ` [PATCH V3 1/8] dt-bindings: mailbox: Add YAML schemas for QCOM APCS global block Sivaprakash Murugesan
@ 2020-04-20 20:59   ` Rob Herring
  2020-05-04  6:14     ` Sivaprakash Murugesan
  0 siblings, 1 reply; 21+ messages in thread
From: Rob Herring @ 2020-04-20 20:59 UTC (permalink / raw)
  To: Sivaprakash Murugesan
  Cc: agross, bjorn.andersson, mturquette, sboyd, jassisinghbrar,
	linux-arm-msm, linux-clk, devicetree, linux-kernel

On Tue, Apr 14, 2020 at 08:25:15AM +0530, Sivaprakash Murugesan wrote:
> Qualcomm APCS global block provides a bunch of generic properties which
> are required in a device tree. Add YAML schema for these properties.
> 
> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
> ---
>  .../bindings/mailbox/qcom,apcs-kpss-global.txt     | 88 ----------------------
>  .../bindings/mailbox/qcom,apcs-kpss-global.yaml    | 88 ++++++++++++++++++++++
>  2 files changed, 88 insertions(+), 88 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt
>  create mode 100644 Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt
> deleted file mode 100644
> index beec612..0000000
> --- a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt
> +++ /dev/null
> @@ -1,88 +0,0 @@
> -Binding for the Qualcomm APCS global block
> -==========================================
> -
> -This binding describes the APCS "global" block found in various Qualcomm
> -platforms.
> -
> -- compatible:
> -	Usage: required
> -	Value type: <string>
> -	Definition: must be one of:
> -		    "qcom,msm8916-apcs-kpss-global",
> -		    "qcom,msm8996-apcs-hmss-global"
> -		    "qcom,msm8998-apcs-hmss-global"
> -		    "qcom,qcs404-apcs-apps-global"
> -		    "qcom,sc7180-apss-shared"
> -		    "qcom,sdm845-apss-shared"
> -		    "qcom,sm8150-apss-shared"
> -		    "qcom,ipq8074-apcs-apps-global"
> -
> -- reg:
> -	Usage: required
> -	Value type: <prop-encoded-array>
> -	Definition: must specify the base address and size of the global block
> -
> -- clocks:
> -	Usage: required if #clock-names property is present
> -	Value type: <phandle array>
> -	Definition: phandles to the two parent clocks of the clock driver.
> -
> -- #mbox-cells:
> -	Usage: required
> -	Value type: <u32>
> -	Definition: as described in mailbox.txt, must be 1
> -
> -- #clock-cells:
> -	Usage: optional
> -	Value type: <u32>
> -	Definition: as described in clock.txt, must be 0
> -
> -- clock-names:
> -	Usage: required if the platform data based clock driver needs to
> -	retrieve the parent clock names from device tree.
> -	This will requires two mandatory clocks to be defined.
> -	Value type: <string-array>
> -	Definition: must be "pll" and "aux"
> -
> -= EXAMPLE
> -The following example describes the APCS HMSS found in MSM8996 and part of the
> -GLINK RPM referencing the "rpm_hlos" doorbell therein.
> -
> -	apcs_glb: mailbox@9820000 {
> -		compatible = "qcom,msm8996-apcs-hmss-global";
> -		reg = <0x9820000 0x1000>;
> -
> -		#mbox-cells = <1>;
> -	};
> -
> -	rpm-glink {
> -		compatible = "qcom,glink-rpm";
> -
> -		interrupts = <GIC_SPI 168 IRQ_TYPE_EDGE_RISING>;
> -
> -		qcom,rpm-msg-ram = <&rpm_msg_ram>;
> -
> -		mboxes = <&apcs_glb 0>;
> -		mbox-names = "rpm_hlos";
> -	};
> -
> -Below is another example of the APCS binding on MSM8916 platforms:
> -
> -	apcs: mailbox@b011000 {
> -		compatible = "qcom,msm8916-apcs-kpss-global";
> -		reg = <0xb011000 0x1000>;
> -		#mbox-cells = <1>;
> -		clocks = <&a53pll>;
> -		#clock-cells = <0>;
> -	};
> -
> -Below is another example of the APCS binding on QCS404 platforms:
> -
> -	apcs_glb: mailbox@b011000 {
> -		compatible = "qcom,qcs404-apcs-apps-global", "syscon";
> -		reg = <0x0b011000 0x1000>;
> -		#mbox-cells = <1>;
> -		clocks = <&apcs_hfpll>, <&gcc GCC_GPLL0_AO_OUT_MAIN>;
> -		clock-names = "pll", "aux";
> -		#clock-cells = <0>;
> -	};
> diff --git a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
> new file mode 100644
> index 0000000..b46474b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
> @@ -0,0 +1,88 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/mailbox/qcom,apcs-kpss-global.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Qualcomm APCS global block bindings
> +
> +description:
> +  This binding describes the APCS "global" block found in various Qualcomm
> +  platforms.
> +
> +maintainers:
> +  - Sivaprakash Murugesan <sivaprak@codeaurora.org>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,ipq8074-apcs-apps-global
> +      - qcom,msm8916-apcs-kpss-global
> +      - qcom,msm8996-apcs-hmss-global
> +      - qcom,msm8998-apcs-hmss-global
> +      - qcom,qcs404-apcs-apps-global
> +      - qcom,sc7180-apss-shared
> +      - qcom,sdm845-apss-shared
> +      - qcom,sm8150-apss-shared
> +
> +  reg:
> +    description: specifies the base address and size of the global block

Can drop this.

> +    maxItems: 1
> +
> +  clocks:
> +    description: phandles to the parent clocks of the clock driver

Need to define how many and what each one is.

> +
> +  '#mbox-cells':
> +    const: 1
> +
> +  '#clock-cells':
> +    const: 0
> +
> +  clock-names:
> +    description:
> +      parent clock names, required if the platform data based clock driver
> +      needs to retrieve the parent clock names from device tree.

Drop.

> +    maxItems: 2

Not needed as 'items' implies this.

> +    items:
> +      - const: pll
> +      - const: aux
> +
> +required:
> +  - compatible
> +  - reg
> +  - '#mbox-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +
> +  # Example apcs with msm8996
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    apcs_glb: mailbox@9820000 {
> +        compatible = "qcom,msm8996-apcs-hmss-global";
> +        reg = <0x9820000 0x1000>;
> +
> +        #mbox-cells = <1>;
> +    };
> +
> +    rpm-glink {
> +        compatible = "qcom,glink-rpm";
> +        interrupts = <GIC_SPI 168 IRQ_TYPE_EDGE_RISING>;
> +        qcom,rpm-msg-ram = <&rpm_msg_ram>;
> +        mboxes = <&apcs_glb 0>;
> +        mbox-names = "rpm_hlos";
> +    };
> +
> +  # Example apcs with qcs404
> +  - |
> +    #define GCC_APSS_AHB_CLK_SRC  1
> +    #define GCC_GPLL0_AO_OUT_MAIN 123
> +    apcs: mailbox@b011000 {
> +        compatible = "qcom,qcs404-apcs-apps-global";
> +        reg = <0x0b011000 0x1000>;
> +        #mbox-cells = <1>;
> +        clocks = <&apcs_hfpll>, <&gcc GCC_GPLL0_AO_OUT_MAIN>;
> +        clock-names = "pll", "aux";
> +        #clock-cells = <0>;
> +    };
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V3 2/8] dt-bindings: clock: Add YAML schemas for QCOM A53 PLL
  2020-04-14  2:55 ` [PATCH V3 2/8] dt-bindings: clock: Add YAML schemas for QCOM A53 PLL Sivaprakash Murugesan
@ 2020-04-20 21:01   ` Rob Herring
  2020-05-04  6:13     ` Sivaprakash Murugesan
  2020-04-20 21:03   ` Rob Herring
  1 sibling, 1 reply; 21+ messages in thread
From: Rob Herring @ 2020-04-20 21:01 UTC (permalink / raw)
  To: Sivaprakash Murugesan
  Cc: agross, bjorn.andersson, mturquette, sboyd, jassisinghbrar,
	linux-arm-msm, linux-clk, devicetree, linux-kernel

On Tue, Apr 14, 2020 at 08:25:16AM +0530, Sivaprakash Murugesan wrote:
> This patch adds schema for primary CPU PLL found on few Qualcomm
> platforms.
> 
> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
> ---
> [V3]
>  * Fixed dt binding error in "$id" field.
> 
>  .../devicetree/bindings/clock/qcom,a53pll.txt      | 22 --------
>  .../devicetree/bindings/clock/qcom,a53pll.yaml     | 60 ++++++++++++++++++++++
>  2 files changed, 60 insertions(+), 22 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/clock/qcom,a53pll.txt
>  create mode 100644 Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,a53pll.txt b/Documentation/devicetree/bindings/clock/qcom,a53pll.txt
> deleted file mode 100644
> index e3fa811..0000000
> --- a/Documentation/devicetree/bindings/clock/qcom,a53pll.txt
> +++ /dev/null
> @@ -1,22 +0,0 @@
> -Qualcomm MSM8916 A53 PLL Binding
> ---------------------------------
> -The A53 PLL on MSM8916 platforms is the main CPU PLL used used for frequencies
> -above 1GHz.
> -
> -Required properties :
> -- compatible : Shall contain only one of the following:
> -
> -		"qcom,msm8916-a53pll"
> -
> -- reg : shall contain base register location and length
> -
> -- #clock-cells : must be set to <0>
> -
> -Example:
> -
> -	a53pll: clock@b016000 {
> -		compatible = "qcom,msm8916-a53pll";
> -		reg = <0xb016000 0x40>;
> -		#clock-cells = <0>;
> -	};
> -
> diff --git a/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml b/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
> new file mode 100644
> index 0000000..c865293
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
> @@ -0,0 +1,60 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/qcom,a53pll.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm A53 PLL Binding
> +
> +maintainers:
> +  - Sivaprakash Murugesan <sivaprak@codeaurora.org>
> +
> +description:
> +  The A53 PLL on few Qualcomm platforms is the main CPU PLL used used for
> +  frequencies above 1GHz.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,msm8916-a53pll
> +      - qcom,ipq6018-a53pll
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#clock-cells':
> +    const: 0
> +
> +  clocks:
> +    description: clocks required for this controller.

That's every 'clocks'. Drop.

> +    maxItems: 1
> +
> +  clock-names:
> +    description: clock output names of required clocks.

Drop. 'clock-names' are the input names.

> +    maxItems: 1

Need to define what the names are.

> +
> +required:
> +  - compatible
> +  - reg
> +  - '#clock-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  #Example 1 - A53 PLL found on MSM8916 devices
> +  - |
> +    a53pll: clock@b016000 {
> +        compatible = "qcom,msm8916-a53pll";
> +        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
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V3 2/8] dt-bindings: clock: Add YAML schemas for QCOM A53 PLL
  2020-04-14  2:55 ` [PATCH V3 2/8] dt-bindings: clock: Add YAML schemas for QCOM A53 PLL Sivaprakash Murugesan
  2020-04-20 21:01   ` Rob Herring
@ 2020-04-20 21:03   ` Rob Herring
  1 sibling, 0 replies; 21+ messages in thread
From: Rob Herring @ 2020-04-20 21:03 UTC (permalink / raw)
  To: Sivaprakash Murugesan
  Cc: agross, bjorn.andersson, mturquette, sboyd, jassisinghbrar,
	linux-arm-msm, linux-clk, devicetree, linux-kernel

On Tue, Apr 14, 2020 at 08:25:16AM +0530, Sivaprakash Murugesan wrote:
> This patch adds schema for primary CPU PLL found on few Qualcomm
> platforms.
> 
> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
> ---
> [V3]
>  * Fixed dt binding error in "$id" field.
> 
>  .../devicetree/bindings/clock/qcom,a53pll.txt      | 22 --------
>  .../devicetree/bindings/clock/qcom,a53pll.yaml     | 60 ++++++++++++++++++++++
>  2 files changed, 60 insertions(+), 22 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/clock/qcom,a53pll.txt
>  create mode 100644 Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,a53pll.txt b/Documentation/devicetree/bindings/clock/qcom,a53pll.txt
> deleted file mode 100644
> index e3fa811..0000000
> --- a/Documentation/devicetree/bindings/clock/qcom,a53pll.txt
> +++ /dev/null
> @@ -1,22 +0,0 @@
> -Qualcomm MSM8916 A53 PLL Binding
> ---------------------------------
> -The A53 PLL on MSM8916 platforms is the main CPU PLL used used for frequencies
> -above 1GHz.
> -
> -Required properties :
> -- compatible : Shall contain only one of the following:
> -
> -		"qcom,msm8916-a53pll"
> -
> -- reg : shall contain base register location and length
> -
> -- #clock-cells : must be set to <0>
> -
> -Example:
> -
> -	a53pll: clock@b016000 {
> -		compatible = "qcom,msm8916-a53pll";
> -		reg = <0xb016000 0x40>;
> -		#clock-cells = <0>;
> -	};
> -
> diff --git a/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml b/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
> new file mode 100644
> index 0000000..c865293
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
> @@ -0,0 +1,60 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/qcom,a53pll.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm A53 PLL Binding
> +
> +maintainers:
> +  - Sivaprakash Murugesan <sivaprak@codeaurora.org>
> +
> +description:
> +  The A53 PLL on few Qualcomm platforms is the main CPU PLL used used for
> +  frequencies above 1GHz.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,msm8916-a53pll
> +      - qcom,ipq6018-a53pll

This new compatible goes in the next patch.

> +
> +  reg:
> +    maxItems: 1
> +
> +  '#clock-cells':
> +    const: 0
> +
> +  clocks:
> +    description: clocks required for this controller.
> +    maxItems: 1
> +
> +  clock-names:
> +    description: clock output names of required clocks.
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - '#clock-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  #Example 1 - A53 PLL found on MSM8916 devices
> +  - |
> +    a53pll: clock@b016000 {
> +        compatible = "qcom,msm8916-a53pll";
> +        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
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V3 6/8] dt-bindings: mailbox: Add dt-bindings for ipq6018 apcs global block
  2020-04-14  2:55 ` [PATCH V3 6/8] dt-bindings: mailbox: Add dt-bindings for ipq6018 apcs global block Sivaprakash Murugesan
@ 2020-04-20 21:05   ` Rob Herring
  0 siblings, 0 replies; 21+ messages in thread
From: Rob Herring @ 2020-04-20 21:05 UTC (permalink / raw)
  To: Sivaprakash Murugesan
  Cc: agross, bjorn.andersson, mturquette, sboyd, robh+dt,
	jassisinghbrar, linux-arm-msm, linux-clk, devicetree,
	linux-kernel, Sivaprakash Murugesan

On Tue, 14 Apr 2020 08:25:20 +0530, Sivaprakash Murugesan wrote:
> Add dt-bindings for ipq6018 mailbox driver
> 
> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
> ---
>  .../bindings/mailbox/qcom,apcs-kpss-global.yaml         | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V3 3/8] clk: qcom: Add A53 PLL support for ipq6018 devices
  2020-04-14  2:55 ` [PATCH V3 3/8] clk: qcom: Add A53 PLL support for ipq6018 devices Sivaprakash Murugesan
@ 2020-04-22  9:00   ` Stephen Boyd
  2020-04-22 10:44     ` Sivaprakash Murugesan
  0 siblings, 1 reply; 21+ messages in thread
From: Stephen Boyd @ 2020-04-22  9:00 UTC (permalink / raw)
  To: Sivaprakash Murugesan, agross, bjorn.andersson, devicetree,
	jassisinghbrar, linux-arm-msm, linux-clk, linux-kernel,
	mturquette, robh+dt
  Cc: Sivaprakash Murugesan

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 <sivaprak@codeaurora.org>
> ---
>  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.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V3 5/8] clk: qcom: Add ipq apss clock controller
  2020-04-14  2:55 ` [PATCH V3 5/8] clk: qcom: Add ipq " Sivaprakash Murugesan
@ 2020-04-22  9:04   ` Stephen Boyd
  2020-05-04  6:10     ` Sivaprakash Murugesan
  0 siblings, 1 reply; 21+ messages in thread
From: Stephen Boyd @ 2020-04-22  9:04 UTC (permalink / raw)
  To: Sivaprakash Murugesan, agross, bjorn.andersson, devicetree,
	jassisinghbrar, linux-arm-msm, linux-clk, linux-kernel,
	mturquette, robh+dt
  Cc: Sivaprakash Murugesan

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 <sivaprak@codeaurora.org>
> ---
>  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",
> +       },
> +};

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V3 3/8] clk: qcom: Add A53 PLL support for ipq6018 devices
  2020-04-22  9:00   ` Stephen Boyd
@ 2020-04-22 10:44     ` Sivaprakash Murugesan
  2020-05-14 20:40       ` Stephen Boyd
  0 siblings, 1 reply; 21+ messages in thread
From: Sivaprakash Murugesan @ 2020-04-22 10:44 UTC (permalink / raw)
  To: Stephen Boyd, agross, bjorn.andersson, devicetree,
	jassisinghbrar, linux-arm-msm, linux-clk, linux-kernel,
	mturquette, robh+dt

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 <sivaprak@codeaurora.org>
>> ---
>>   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.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V3 5/8] clk: qcom: Add ipq apss clock controller
  2020-04-22  9:04   ` Stephen Boyd
@ 2020-05-04  6:10     ` Sivaprakash Murugesan
  0 siblings, 0 replies; 21+ messages in thread
From: Sivaprakash Murugesan @ 2020-05-04  6:10 UTC (permalink / raw)
  To: Stephen Boyd, agross, bjorn.andersson, devicetree,
	jassisinghbrar, linux-arm-msm, linux-clk, linux-kernel,
	mturquette, robh+dt

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 <sivaprak@codeaurora.org>
>> ---
>>   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.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V3 2/8] dt-bindings: clock: Add YAML schemas for QCOM A53 PLL
  2020-04-20 21:01   ` Rob Herring
@ 2020-05-04  6:13     ` Sivaprakash Murugesan
  0 siblings, 0 replies; 21+ messages in thread
From: Sivaprakash Murugesan @ 2020-05-04  6:13 UTC (permalink / raw)
  To: Rob Herring
  Cc: agross, bjorn.andersson, mturquette, sboyd, jassisinghbrar,
	linux-arm-msm, linux-clk, devicetree, linux-kernel

Hi Rob,

On 4/21/2020 2:31 AM, Rob Herring wrote:
> On Tue, Apr 14, 2020 at 08:25:16AM +0530, Sivaprakash Murugesan wrote:
>> This patch adds schema for primary CPU PLL found on few Qualcomm
>> platforms.
>>
>> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
>> ---
>> [V3]
>>   * Fixed dt binding error in "$id" field.
>>
>>   .../devicetree/bindings/clock/qcom,a53pll.txt      | 22 --------
>>   .../devicetree/bindings/clock/qcom,a53pll.yaml     | 60 ++++++++++++++++++++++
>>   2 files changed, 60 insertions(+), 22 deletions(-)
>>   delete mode 100644 Documentation/devicetree/bindings/clock/qcom,a53pll.txt
>>   create mode 100644 Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,a53pll.txt b/Documentation/devicetree/bindings/clock/qcom,a53pll.txt
>> deleted file mode 100644
>> index e3fa811..0000000
>> --- a/Documentation/devicetree/bindings/clock/qcom,a53pll.txt
>> +++ /dev/null
>> @@ -1,22 +0,0 @@
>> -Qualcomm MSM8916 A53 PLL Binding
>> ---------------------------------
>> -The A53 PLL on MSM8916 platforms is the main CPU PLL used used for frequencies
>> -above 1GHz.
>> -
>> -Required properties :
>> -- compatible : Shall contain only one of the following:
>> -
>> -		"qcom,msm8916-a53pll"
>> -
>> -- reg : shall contain base register location and length
>> -
>> -- #clock-cells : must be set to <0>
>> -
>> -Example:
>> -
>> -	a53pll: clock@b016000 {
>> -		compatible = "qcom,msm8916-a53pll";
>> -		reg = <0xb016000 0x40>;
>> -		#clock-cells = <0>;
>> -	};
>> -
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml b/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
>> new file mode 100644
>> index 0000000..c865293
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
>> @@ -0,0 +1,60 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/clock/qcom,a53pll.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm A53 PLL Binding
>> +
>> +maintainers:
>> +  - Sivaprakash Murugesan <sivaprak@codeaurora.org>
>> +
>> +description:
>> +  The A53 PLL on few Qualcomm platforms is the main CPU PLL used used for
>> +  frequencies above 1GHz.
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - qcom,msm8916-a53pll
>> +      - qcom,ipq6018-a53pll
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  '#clock-cells':
>> +    const: 0
>> +
>> +  clocks:
>> +    description: clocks required for this controller.
> That's every 'clocks'. Drop.
ok.
>
>> +    maxItems: 1
>> +
>> +  clock-names:
>> +    description: clock output names of required clocks.
> Drop. 'clock-names' are the input names.
ok.
>
>> +    maxItems: 1
> Need to define what the names are.

ok.

>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V3 1/8] dt-bindings: mailbox: Add YAML schemas for QCOM APCS global block
  2020-04-20 20:59   ` Rob Herring
@ 2020-05-04  6:14     ` Sivaprakash Murugesan
  0 siblings, 0 replies; 21+ messages in thread
From: Sivaprakash Murugesan @ 2020-05-04  6:14 UTC (permalink / raw)
  To: Rob Herring
  Cc: agross, bjorn.andersson, mturquette, sboyd, jassisinghbrar,
	linux-arm-msm, linux-clk, devicetree, linux-kernel


On 4/21/2020 2:29 AM, Rob Herring wrote:
> On Tue, Apr 14, 2020 at 08:25:15AM +0530, Sivaprakash Murugesan wrote:
>> Qualcomm APCS global block provides a bunch of generic properties which
>> are required in a device tree. Add YAML schema for these properties.
>>
>> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
>> ---
>>   .../bindings/mailbox/qcom,apcs-kpss-global.txt     | 88 ----------------------
>>   .../bindings/mailbox/qcom,apcs-kpss-global.yaml    | 88 ++++++++++++++++++++++
>>   2 files changed, 88 insertions(+), 88 deletions(-)
>>   delete mode 100644 Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt
>>   create mode 100644 Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt
>> deleted file mode 100644
>> index beec612..0000000
>> --- a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt
>> +++ /dev/null
>> @@ -1,88 +0,0 @@
>> -Binding for the Qualcomm APCS global block
>> -==========================================
>> -
>> -This binding describes the APCS "global" block found in various Qualcomm
>> -platforms.
>> -
>> -- compatible:
>> -	Usage: required
>> -	Value type: <string>
>> -	Definition: must be one of:
>> -		    "qcom,msm8916-apcs-kpss-global",
>> -		    "qcom,msm8996-apcs-hmss-global"
>> -		    "qcom,msm8998-apcs-hmss-global"
>> -		    "qcom,qcs404-apcs-apps-global"
>> -		    "qcom,sc7180-apss-shared"
>> -		    "qcom,sdm845-apss-shared"
>> -		    "qcom,sm8150-apss-shared"
>> -		    "qcom,ipq8074-apcs-apps-global"
>> -
>> -- reg:
>> -	Usage: required
>> -	Value type: <prop-encoded-array>
>> -	Definition: must specify the base address and size of the global block
>> -
>> -- clocks:
>> -	Usage: required if #clock-names property is present
>> -	Value type: <phandle array>
>> -	Definition: phandles to the two parent clocks of the clock driver.
>> -
>> -- #mbox-cells:
>> -	Usage: required
>> -	Value type: <u32>
>> -	Definition: as described in mailbox.txt, must be 1
>> -
>> -- #clock-cells:
>> -	Usage: optional
>> -	Value type: <u32>
>> -	Definition: as described in clock.txt, must be 0
>> -
>> -- clock-names:
>> -	Usage: required if the platform data based clock driver needs to
>> -	retrieve the parent clock names from device tree.
>> -	This will requires two mandatory clocks to be defined.
>> -	Value type: <string-array>
>> -	Definition: must be "pll" and "aux"
>> -
>> -= EXAMPLE
>> -The following example describes the APCS HMSS found in MSM8996 and part of the
>> -GLINK RPM referencing the "rpm_hlos" doorbell therein.
>> -
>> -	apcs_glb: mailbox@9820000 {
>> -		compatible = "qcom,msm8996-apcs-hmss-global";
>> -		reg = <0x9820000 0x1000>;
>> -
>> -		#mbox-cells = <1>;
>> -	};
>> -
>> -	rpm-glink {
>> -		compatible = "qcom,glink-rpm";
>> -
>> -		interrupts = <GIC_SPI 168 IRQ_TYPE_EDGE_RISING>;
>> -
>> -		qcom,rpm-msg-ram = <&rpm_msg_ram>;
>> -
>> -		mboxes = <&apcs_glb 0>;
>> -		mbox-names = "rpm_hlos";
>> -	};
>> -
>> -Below is another example of the APCS binding on MSM8916 platforms:
>> -
>> -	apcs: mailbox@b011000 {
>> -		compatible = "qcom,msm8916-apcs-kpss-global";
>> -		reg = <0xb011000 0x1000>;
>> -		#mbox-cells = <1>;
>> -		clocks = <&a53pll>;
>> -		#clock-cells = <0>;
>> -	};
>> -
>> -Below is another example of the APCS binding on QCS404 platforms:
>> -
>> -	apcs_glb: mailbox@b011000 {
>> -		compatible = "qcom,qcs404-apcs-apps-global", "syscon";
>> -		reg = <0x0b011000 0x1000>;
>> -		#mbox-cells = <1>;
>> -		clocks = <&apcs_hfpll>, <&gcc GCC_GPLL0_AO_OUT_MAIN>;
>> -		clock-names = "pll", "aux";
>> -		#clock-cells = <0>;
>> -	};
>> diff --git a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
>> new file mode 100644
>> index 0000000..b46474b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
>> @@ -0,0 +1,88 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: "http://devicetree.org/schemas/mailbox/qcom,apcs-kpss-global.yaml#"
>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
>> +
>> +title: Qualcomm APCS global block bindings
>> +
>> +description:
>> +  This binding describes the APCS "global" block found in various Qualcomm
>> +  platforms.
>> +
>> +maintainers:
>> +  - Sivaprakash Murugesan <sivaprak@codeaurora.org>
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - qcom,ipq8074-apcs-apps-global
>> +      - qcom,msm8916-apcs-kpss-global
>> +      - qcom,msm8996-apcs-hmss-global
>> +      - qcom,msm8998-apcs-hmss-global
>> +      - qcom,qcs404-apcs-apps-global
>> +      - qcom,sc7180-apss-shared
>> +      - qcom,sdm845-apss-shared
>> +      - qcom,sm8150-apss-shared
>> +
>> +  reg:
>> +    description: specifies the base address and size of the global block
> Can drop this.
ok.
>
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    description: phandles to the parent clocks of the clock driver
> Need to define how many and what each one is.
ok.
>
>> +
>> +  '#mbox-cells':
>> +    const: 1
>> +
>> +  '#clock-cells':
>> +    const: 0
>> +
>> +  clock-names:
>> +    description:
>> +      parent clock names, required if the platform data based clock driver
>> +      needs to retrieve the parent clock names from device tree.
> Drop.
ok.
>
>> +    maxItems: 2
> Not needed as 'items' implies this.
ok.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V3 3/8] clk: qcom: Add A53 PLL support for ipq6018 devices
  2020-04-22 10:44     ` Sivaprakash Murugesan
@ 2020-05-14 20:40       ` Stephen Boyd
  2020-05-24  9:36         ` Sivaprakash Murugesan
  0 siblings, 1 reply; 21+ messages in thread
From: Stephen Boyd @ 2020-05-14 20:40 UTC (permalink / raw)
  To: Sivaprakash Murugesan, agross, bjorn.andersson, devicetree,
	jassisinghbrar, linux-arm-msm, linux-clk, linux-kernel,
	mturquette, robh+dt

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?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V3 3/8] clk: qcom: Add A53 PLL support for ipq6018 devices
  2020-05-14 20:40       ` Stephen Boyd
@ 2020-05-24  9:36         ` Sivaprakash Murugesan
  0 siblings, 0 replies; 21+ messages in thread
From: Sivaprakash Murugesan @ 2020-05-24  9:36 UTC (permalink / raw)
  To: Stephen Boyd, agross, bjorn.andersson, devicetree,
	jassisinghbrar, linux-arm-msm, linux-clk, linux-kernel,
	mturquette, robh+dt


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?


^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2020-05-24  9:36 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-14  2:55 [PATCH V3 0/8] Add APSS clock controller support for IPQ6018 Sivaprakash Murugesan
2020-04-14  2:55 ` [PATCH V3 1/8] dt-bindings: mailbox: Add YAML schemas for QCOM APCS global block Sivaprakash Murugesan
2020-04-20 20:59   ` Rob Herring
2020-05-04  6:14     ` Sivaprakash Murugesan
2020-04-14  2:55 ` [PATCH V3 2/8] dt-bindings: clock: Add YAML schemas for QCOM A53 PLL Sivaprakash Murugesan
2020-04-20 21:01   ` Rob Herring
2020-05-04  6:13     ` Sivaprakash Murugesan
2020-04-20 21:03   ` Rob Herring
2020-04-14  2:55 ` [PATCH V3 3/8] clk: qcom: Add A53 PLL support for ipq6018 devices Sivaprakash Murugesan
2020-04-22  9:00   ` Stephen Boyd
2020-04-22 10:44     ` Sivaprakash Murugesan
2020-05-14 20:40       ` Stephen Boyd
2020-05-24  9:36         ` Sivaprakash Murugesan
2020-04-14  2:55 ` [PATCH V3 4/8] clk: qcom: Add DT bindings for ipq6018 apss clock controller Sivaprakash Murugesan
2020-04-14  2:55 ` [PATCH V3 5/8] clk: qcom: Add ipq " Sivaprakash Murugesan
2020-04-22  9:04   ` Stephen Boyd
2020-05-04  6:10     ` Sivaprakash Murugesan
2020-04-14  2:55 ` [PATCH V3 6/8] dt-bindings: mailbox: Add dt-bindings for ipq6018 apcs global block Sivaprakash Murugesan
2020-04-20 21:05   ` Rob Herring
2020-04-14  2:55 ` [PATCH V3 7/8] mailbox: qcom: Add ipq6018 apcs compatible Sivaprakash Murugesan
2020-04-14  2:55 ` [PATCH V3 8/8] arm64: dts: ipq6018: Add a53 pll and apcs clock Sivaprakash Murugesan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).