devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v11 0/6] Add support for Qualcomm A53 CPU clock
@ 2017-12-05 15:46 Georgi Djakov
  2017-12-05 15:46 ` [PATCH v11 1/6] mailbox: qcom: Convert APCS IPC driver to use regmap Georgi Djakov
                   ` (8 more replies)
  0 siblings, 9 replies; 27+ messages in thread
From: Georgi Djakov @ 2017-12-05 15:46 UTC (permalink / raw)
  To: sboyd, jassisinghbrar, bjorn.andersson
  Cc: mturquette, robh, linux-clk, linux-kernel, linux-arm-msm,
	devicetree, georgi.djakov

This patchset adds support for the A53 CPU clock on MSM8916 platforms
and allows scaling of the CPU frequency on msm8916 based platforms.

Changes since v10 (https://lkml.org/lkml/2017/12/1/577)
* Addressed Bjorn's comments on APCS clock driver.
* Picked Acks from Rob and Bjorn.

Changes since v9 (https://lkml.org/lkml/2017/9/21/511)
* Added the clock properties to the APCS DT node, instead of adding a subnode
and also replaced patch "mailbox: qcom: Populate APCS child platform devices"
with "mailbox: qcom: Create APCS child device for clock controller".
* Dropped patch "mailbox: qcom: Move the apcs struct into a separate header",
and use dev_get_regmap(dev->parent) in the child driver.
* Addressed Bjorn's comments on a53-pll and apcs-clk drivers.
* Added SPDX copyright identifiers.

Changes since v8 (https://lkml.org/lkml/2017/6/23/476)
 * Converted APCS mailbox driver to use regmap and to populate child
 platform devices that will handle the rest of the functionality
 provided by APCS block.
 * Picked Rob's Ack for the PLL binding.
 * Changed the APCS binding and put it into a separate patch.
 * Addressed review comments.
 * Minor changes.

Changes since v7 (https://lkml.org/lkml/2016/10/31/296)
 * Add the APCS clock controller to the APCS driver to expose both the
 mailbox and clock controller functionality as discussed earlier:
 https://lkml.org/lkml/2016/11/14/860
 * Changed the a53pll compatible string as suggested by Rob.

Changes since v6 (https://lkml.org/lkml/2016/9/7/347)
 * Addressed various comments from Stephen Boyd

Changes since v5 (https://lkml.org/lkml/2016/2/1/407)
 * Rebase to clk-next and update according to the recent API changes.

Changes since v4 (https://lkml.org/lkml/2015/12/14/367)
 * Convert to builtin drivers as now __clk_lookup() is used

Changes since v3 (https://lkml.org/lkml/2015/8/12/585)
 * Split driver into two parts - and separate A53 PLL and
   A53 clock controller drivers.
 * Drop the safe switch hook patch. Add a clock notifier in
   the clock provider to handle switching via safe mux and
   divider configuration.

Changes since v2 (https://lkml.org/lkml/2015/7/24/526)
 * Drop gpll0_vote patch.
 * Switch to the new clk_hw_* APIs.
 * Rebase to the current clk-next.

Changes since v1 (https://lkml.org/lkml/2015/6/12/193)
 * Drop SR2 PLL patch, as it is already applied.
 * Add gpll0_vote rate propagation patch.
 * Update/rebase patches to the current clk-next.

Georgi Djakov (6):
  mailbox: qcom: Convert APCS IPC driver to use regmap
  mailbox: qcom: Create APCS child device for clock controller
  clk: qcom: Add A53 PLL support
  clk: qcom: Add regmap mux-div clocks support
  dt-bindings: mailbox: qcom: Document the APCS clock binding
  clk: qcom: Add APCS clock controller support

 .../devicetree/bindings/clock/qcom,a53pll.txt      |  22 ++
 .../bindings/mailbox/qcom,apcs-kpss-global.txt     |  18 ++
 drivers/clk/qcom/Kconfig                           |  21 ++
 drivers/clk/qcom/Makefile                          |   3 +
 drivers/clk/qcom/a53-pll.c                         | 110 ++++++++++
 drivers/clk/qcom/apcs-msm8916.c                    | 149 ++++++++++++++
 drivers/clk/qcom/clk-regmap-mux-div.c              | 229 +++++++++++++++++++++
 drivers/clk/qcom/clk-regmap-mux-div.h              |  46 +++++
 drivers/mailbox/qcom-apcs-ipc-mailbox.c            |  35 +++-
 9 files changed, 628 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,a53pll.txt
 create mode 100644 drivers/clk/qcom/a53-pll.c
 create mode 100644 drivers/clk/qcom/apcs-msm8916.c
 create mode 100644 drivers/clk/qcom/clk-regmap-mux-div.c
 create mode 100644 drivers/clk/qcom/clk-regmap-mux-div.h


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

* [PATCH v11 1/6] mailbox: qcom: Convert APCS IPC driver to use regmap
  2017-12-05 15:46 [PATCH v11 0/6] Add support for Qualcomm A53 CPU clock Georgi Djakov
@ 2017-12-05 15:46 ` Georgi Djakov
  2017-12-05 15:46 ` [PATCH v11 2/6] mailbox: qcom: Create APCS child device for clock controller Georgi Djakov
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 27+ messages in thread
From: Georgi Djakov @ 2017-12-05 15:46 UTC (permalink / raw)
  To: sboyd, jassisinghbrar, bjorn.andersson
  Cc: mturquette, robh, linux-clk, linux-kernel, linux-arm-msm,
	devicetree, georgi.djakov

This hardware block provides more functionalities that just IPC. Convert
it to regmap to allow other child platform devices to use the same regmap.

Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/mailbox/qcom-apcs-ipc-mailbox.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/mailbox/qcom-apcs-ipc-mailbox.c b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
index 9924c6d7f05d..ab344bc6fa63 100644
--- a/drivers/mailbox/qcom-apcs-ipc-mailbox.c
+++ b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
@@ -18,6 +18,7 @@
 #include <linux/of.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
+#include <linux/regmap.h>
 #include <linux/mailbox_controller.h>
 
 #define QCOM_APCS_IPC_BITS	32
@@ -26,19 +27,25 @@ struct qcom_apcs_ipc {
 	struct mbox_controller mbox;
 	struct mbox_chan mbox_chans[QCOM_APCS_IPC_BITS];
 
-	void __iomem *reg;
+	struct regmap *regmap;
 	unsigned long offset;
 };
 
+static const struct regmap_config apcs_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.max_register = 0x1000,
+	.fast_io = true,
+};
+
 static int qcom_apcs_ipc_send_data(struct mbox_chan *chan, void *data)
 {
 	struct qcom_apcs_ipc *apcs = container_of(chan->mbox,
 						  struct qcom_apcs_ipc, mbox);
 	unsigned long idx = (unsigned long)chan->con_priv;
 
-	writel(BIT(idx), apcs->reg);
-
-	return 0;
+	return regmap_write(apcs->regmap, apcs->offset, BIT(idx));
 }
 
 static const struct mbox_chan_ops qcom_apcs_ipc_ops = {
@@ -47,7 +54,9 @@ static const struct mbox_chan_ops qcom_apcs_ipc_ops = {
 
 static int qcom_apcs_ipc_probe(struct platform_device *pdev)
 {
+	struct device_node *np = pdev->dev.of_node;
 	struct qcom_apcs_ipc *apcs;
+	struct regmap *regmap;
 	struct resource *res;
 	unsigned long offset;
 	void __iomem *base;
@@ -63,9 +72,14 @@ static int qcom_apcs_ipc_probe(struct platform_device *pdev)
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
+	regmap = devm_regmap_init_mmio(&pdev->dev, base, &apcs_regmap_config);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
 	offset = (unsigned long)of_device_get_match_data(&pdev->dev);
 
-	apcs->reg = base + offset;
+	apcs->regmap = regmap;
+	apcs->offset = offset;
 
 	/* Initialize channel identifiers */
 	for (i = 0; i < ARRAY_SIZE(apcs->mbox_chans); i++)

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

* [PATCH v11 2/6] mailbox: qcom: Create APCS child device for clock controller
  2017-12-05 15:46 [PATCH v11 0/6] Add support for Qualcomm A53 CPU clock Georgi Djakov
  2017-12-05 15:46 ` [PATCH v11 1/6] mailbox: qcom: Convert APCS IPC driver to use regmap Georgi Djakov
@ 2017-12-05 15:46 ` Georgi Djakov
  2017-12-23  4:57   ` Jassi Brar
  2017-12-05 15:46 ` [PATCH v11 3/6] clk: qcom: Add A53 PLL support Georgi Djakov
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 27+ messages in thread
From: Georgi Djakov @ 2017-12-05 15:46 UTC (permalink / raw)
  To: sboyd, jassisinghbrar, bjorn.andersson
  Cc: mturquette, robh, linux-clk, linux-kernel, linux-arm-msm,
	devicetree, georgi.djakov

There is a clock controller functionality provided by the APCS hardware
block of msm8916 devices. The device-tree would represent an APCS node
with both mailbox and clock provider properties.
Create a platform child device for the clock controller functionality so
the driver can probe and use APCS as parent.

Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/mailbox/qcom-apcs-ipc-mailbox.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/mailbox/qcom-apcs-ipc-mailbox.c b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
index ab344bc6fa63..57bde0dfd12f 100644
--- a/drivers/mailbox/qcom-apcs-ipc-mailbox.c
+++ b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
@@ -29,6 +29,7 @@ struct qcom_apcs_ipc {
 
 	struct regmap *regmap;
 	unsigned long offset;
+	struct platform_device *clk;
 };
 
 static const struct regmap_config apcs_regmap_config = {
@@ -96,6 +97,14 @@ static int qcom_apcs_ipc_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	if (of_device_is_compatible(np, "qcom,msm8916-apcs-kpss-global")) {
+		apcs->clk = platform_device_register_data(&pdev->dev,
+							  "qcom-apcs-msm8916-clk",
+							  -1, NULL, 0);
+		if (IS_ERR(apcs->clk))
+			dev_err(&pdev->dev, "failed to register APCS clk\n");
+	}
+
 	platform_set_drvdata(pdev, apcs);
 
 	return 0;
@@ -104,8 +113,10 @@ static int qcom_apcs_ipc_probe(struct platform_device *pdev)
 static int qcom_apcs_ipc_remove(struct platform_device *pdev)
 {
 	struct qcom_apcs_ipc *apcs = platform_get_drvdata(pdev);
+	struct platform_device *clk = apcs->clk;
 
 	mbox_controller_unregister(&apcs->mbox);
+	platform_device_unregister(clk);
 
 	return 0;
 }

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

* [PATCH v11 3/6] clk: qcom: Add A53 PLL support
  2017-12-05 15:46 [PATCH v11 0/6] Add support for Qualcomm A53 CPU clock Georgi Djakov
  2017-12-05 15:46 ` [PATCH v11 1/6] mailbox: qcom: Convert APCS IPC driver to use regmap Georgi Djakov
  2017-12-05 15:46 ` [PATCH v11 2/6] mailbox: qcom: Create APCS child device for clock controller Georgi Djakov
@ 2017-12-05 15:46 ` Georgi Djakov
       [not found]   ` <20171205154701.27730-4-georgi.djakov-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
  2017-12-05 15:46 ` [PATCH v11 4/6] clk: qcom: Add regmap mux-div clocks support Georgi Djakov
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 27+ messages in thread
From: Georgi Djakov @ 2017-12-05 15:46 UTC (permalink / raw)
  To: sboyd, jassisinghbrar, bjorn.andersson
  Cc: mturquette, robh, linux-clk, linux-kernel, linux-arm-msm,
	devicetree, georgi.djakov

The CPUs on Qualcomm MSM8916-based platforms are clocked by two PLLs,
a primary (A53) CPU PLL and a secondary fixed-rate GPLL0. These sources
are connected to a mux and half-integer divider, which is feeding the
CPU cores.

This patch adds support for the primary CPU PLL which generates the
higher range of frequencies above 1GHz.

Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 .../devicetree/bindings/clock/qcom,a53pll.txt      |  22 +++++
 drivers/clk/qcom/Kconfig                           |  10 ++
 drivers/clk/qcom/Makefile                          |   1 +
 drivers/clk/qcom/a53-pll.c                         | 110 +++++++++++++++++++++
 4 files changed, 143 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,a53pll.txt
 create mode 100644 drivers/clk/qcom/a53-pll.c

diff --git a/Documentation/devicetree/bindings/clock/qcom,a53pll.txt b/Documentation/devicetree/bindings/clock/qcom,a53pll.txt
new file mode 100644
index 000000000000..e3fa8118eaee
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/qcom,a53pll.txt
@@ -0,0 +1,22 @@
+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/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index 9f6c278deead..81ac7b9378fe 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -12,6 +12,16 @@ config COMMON_CLK_QCOM
 	select REGMAP_MMIO
 	select RESET_CONTROLLER
 
+config QCOM_A53PLL
+	bool "MSM8916 A53 PLL"
+	depends on COMMON_CLK_QCOM
+	default ARCH_QCOM
+	help
+	  Support for the A53 PLL on MSM8916 devices. It provides
+	  the CPU with frequencies above 1GHz.
+	  Say Y if you want to support higher CPU frequencies on MSM8916
+	  devices.
+
 config QCOM_CLK_RPM
 	tristate "RPM based Clock Controller"
 	depends on COMMON_CLK_QCOM && MFD_QCOM_RPM
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index 26410d31446b..e767c60c24ec 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -32,5 +32,6 @@ obj-$(CONFIG_MSM_LCC_8960) += lcc-msm8960.o
 obj-$(CONFIG_MSM_MMCC_8960) += mmcc-msm8960.o
 obj-$(CONFIG_MSM_MMCC_8974) += mmcc-msm8974.o
 obj-$(CONFIG_MSM_MMCC_8996) += mmcc-msm8996.o
+obj-$(CONFIG_QCOM_A53PLL) += a53-pll.o
 obj-$(CONFIG_QCOM_CLK_RPM) += clk-rpm.o
 obj-$(CONFIG_QCOM_CLK_SMD_RPM) += clk-smd-rpm.o
diff --git a/drivers/clk/qcom/a53-pll.c b/drivers/clk/qcom/a53-pll.c
new file mode 100644
index 000000000000..c18cb7614e42
--- /dev/null
+++ b/drivers/clk/qcom/a53-pll.c
@@ -0,0 +1,110 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Qualcomm A53 PLL driver
+ *
+ * Copyright (c) 2017, Linaro Limited
+ * Author: Georgi Djakov <georgi.djakov@linaro.org>
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include "clk-pll.h"
+#include "clk-regmap.h"
+
+static const struct pll_freq_tbl a53pll_freq[] = {
+	{  998400000, 52, 0x0, 0x1, 0 },
+	{ 1094400000, 57, 0x0, 0x1, 0 },
+	{ 1152000000, 62, 0x0, 0x1, 0 },
+	{ 1209600000, 63, 0x0, 0x1, 0 },
+	{ 1248000000, 65, 0x0, 0x1, 0 },
+	{ 1363200000, 71, 0x0, 0x1, 0 },
+	{ 1401600000, 73, 0x0, 0x1, 0 },
+};
+
+static const struct regmap_config a53pll_regmap_config = {
+	.reg_bits		= 32,
+	.reg_stride		= 4,
+	.val_bits		= 32,
+	.max_register		= 0x40,
+	.fast_io		= true,
+};
+
+static int qcom_a53pll_remove(struct platform_device *pdev)
+{
+	of_clk_del_provider(pdev->dev.of_node);
+	return 0;
+}
+
+static int qcom_a53pll_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct regmap *regmap;
+	struct resource *res;
+	struct clk_pll *pll;
+	void __iomem *base;
+	struct clk_init_data init = { };
+	int ret;
+
+	pll = devm_kzalloc(dev, sizeof(*pll), GFP_KERNEL);
+	if (!pll)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	regmap = devm_regmap_init_mmio(dev, base, &a53pll_regmap_config);
+	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 (ret) {
+		dev_err(dev, "failed to register regmap clock: %d\n", ret);
+		return ret;
+	}
+
+	ret = of_clk_add_hw_provider(dev->of_node, of_clk_hw_simple_get,
+				     &pll->clkr.hw);
+	if (ret) {
+		dev_err(dev, "failed to add clock provider: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id qcom_a53pll_match_table[] = {
+	{ .compatible = "qcom,msm8916-a53pll" },
+	{ }
+};
+
+static struct platform_driver qcom_a53pll_driver = {
+	.probe = qcom_a53pll_probe,
+	.remove = qcom_a53pll_remove,
+	.driver = {
+		.name = "qcom-a53pll",
+		.of_match_table = qcom_a53pll_match_table,
+	},
+};
+
+builtin_platform_driver(qcom_a53pll_driver);

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

* [PATCH v11 4/6] clk: qcom: Add regmap mux-div clocks support
  2017-12-05 15:46 [PATCH v11 0/6] Add support for Qualcomm A53 CPU clock Georgi Djakov
                   ` (2 preceding siblings ...)
  2017-12-05 15:46 ` [PATCH v11 3/6] clk: qcom: Add A53 PLL support Georgi Djakov
@ 2017-12-05 15:46 ` Georgi Djakov
  2017-12-29  0:01   ` Stephen Boyd
  2017-12-05 15:47 ` [PATCH v11 5/6] dt-bindings: mailbox: qcom: Document the APCS clock binding Georgi Djakov
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 27+ messages in thread
From: Georgi Djakov @ 2017-12-05 15:46 UTC (permalink / raw)
  To: sboyd, jassisinghbrar, bjorn.andersson
  Cc: mturquette, robh, linux-clk, linux-kernel, linux-arm-msm,
	devicetree, georgi.djakov

Add support for hardware that can switch both parent clock and divider
at the same time. This avoids generating intermediate frequencies from
either the old parent clock and new divider or new parent clock and
old divider combinations.

Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
---
 drivers/clk/qcom/Makefile             |   1 +
 drivers/clk/qcom/clk-regmap-mux-div.c | 229 ++++++++++++++++++++++++++++++++++
 drivers/clk/qcom/clk-regmap-mux-div.h |  46 +++++++
 3 files changed, 276 insertions(+)
 create mode 100644 drivers/clk/qcom/clk-regmap-mux-div.c
 create mode 100644 drivers/clk/qcom/clk-regmap-mux-div.h

diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index e767c60c24ec..7c51d877f967 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -10,6 +10,7 @@ clk-qcom-y += clk-rcg2.o
 clk-qcom-y += clk-branch.o
 clk-qcom-y += clk-regmap-divider.o
 clk-qcom-y += clk-regmap-mux.o
+clk-qcom-y += clk-regmap-mux-div.o
 clk-qcom-y += reset.o
 clk-qcom-$(CONFIG_QCOM_GDSC) += gdsc.o
 
diff --git a/drivers/clk/qcom/clk-regmap-mux-div.c b/drivers/clk/qcom/clk-regmap-mux-div.c
new file mode 100644
index 000000000000..c5d6fff29598
--- /dev/null
+++ b/drivers/clk/qcom/clk-regmap-mux-div.c
@@ -0,0 +1,229 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2017, Linaro Limited
+ * Author: Georgi Djakov <georgi.djakov@linaro.org>
+ */
+
+#include <linux/bitops.h>
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include <linux/regmap.h>
+
+#include "clk-regmap-mux-div.h"
+
+#define CMD_RCGR			0x0
+#define CMD_RCGR_UPDATE			BIT(0)
+#define CMD_RCGR_DIRTY_CFG		BIT(4)
+#define CMD_RCGR_ROOT_OFF		BIT(31)
+#define CFG_RCGR			0x4
+
+#define to_clk_regmap_mux_div(_hw) \
+	container_of(to_clk_regmap(_hw), struct clk_regmap_mux_div, clkr)
+
+int __mux_div_set_src_div(struct clk_regmap_mux_div *md, u32 src, u32 div)
+{
+	int ret, count;
+	u32 val, mask;
+	const char *name = clk_hw_get_name(&md->clkr.hw);
+
+	val = (div << md->hid_shift) | (src << md->src_shift);
+	mask = ((BIT(md->hid_width) - 1) << md->hid_shift) |
+	       ((BIT(md->src_width) - 1) << md->src_shift);
+
+	ret = regmap_update_bits(md->clkr.regmap, CFG_RCGR + md->reg_offset,
+				 mask, val);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(md->clkr.regmap, CMD_RCGR + md->reg_offset,
+				 CMD_RCGR_UPDATE, CMD_RCGR_UPDATE);
+	if (ret)
+		return ret;
+
+	// Wait for update to take effect
+	for (count = 500; count > 0; count--) {
+		ret = regmap_read(md->clkr.regmap, CMD_RCGR + md->reg_offset,
+				  &val);
+		if (ret)
+			return ret;
+		if (!(val & CMD_RCGR_UPDATE))
+			return 0;
+		udelay(1);
+	}
+
+	pr_err("%s: RCG did not update its configuration", name);
+	return -EBUSY;
+}
+
+static void __mux_div_get_src_div(struct clk_regmap_mux_div *md, u32 *src,
+				  u32 *div)
+{
+	u32 val, d, s;
+	const char *name = clk_hw_get_name(&md->clkr.hw);
+
+	regmap_read(md->clkr.regmap, CMD_RCGR + md->reg_offset, &val);
+
+	if (val & CMD_RCGR_DIRTY_CFG) {
+		pr_err("%s: RCG configuration is pending\n", name);
+		return;
+	}
+
+	regmap_read(md->clkr.regmap, CFG_RCGR + md->reg_offset, &val);
+	s = (val >> md->src_shift);
+	s &= BIT(md->src_width) - 1;
+	*src = s;
+
+	d = (val >> md->hid_shift);
+	d &= BIT(md->hid_width) - 1;
+	*div = d;
+}
+
+static inline bool is_better_rate(unsigned long req, unsigned long best,
+				  unsigned long new)
+{
+	return (req <= new && new < best) || (best < req && best < new);
+}
+
+static int mux_div_determine_rate(struct clk_hw *hw,
+				  struct clk_rate_request *req)
+{
+	struct clk_regmap_mux_div *md = to_clk_regmap_mux_div(hw);
+	unsigned int i, div, max_div;
+	unsigned long actual_rate, best_rate = 0;
+	unsigned long req_rate = req->rate;
+
+	for (i = 0; i < clk_hw_get_num_parents(hw); i++) {
+		struct clk_hw *parent = clk_hw_get_parent_by_index(hw, i);
+		unsigned long parent_rate = clk_hw_get_rate(parent);
+
+		max_div = BIT(md->hid_width) - 1;
+		for (div = 1; div < max_div; div++) {
+			parent_rate = mult_frac(req_rate, div, 2);
+			parent_rate = clk_hw_round_rate(parent, parent_rate);
+			actual_rate = mult_frac(parent_rate, 2, div);
+
+			if (is_better_rate(req_rate, best_rate, actual_rate)) {
+				best_rate = actual_rate;
+				req->rate = best_rate;
+				req->best_parent_rate = parent_rate;
+				req->best_parent_hw = parent;
+			}
+
+			if (actual_rate < req_rate || best_rate <= req_rate)
+				break;
+		}
+	}
+
+	if (!best_rate)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int __mux_div_set_rate_and_parent(struct clk_hw *hw, unsigned long rate,
+					 unsigned long prate, u32 src)
+{
+	struct clk_regmap_mux_div *md = to_clk_regmap_mux_div(hw);
+	int ret;
+	u32 div, max_div, best_src = 0, best_div = 0;
+	unsigned int i;
+	unsigned long actual_rate, best_rate = 0;
+
+	for (i = 0; i < clk_hw_get_num_parents(hw); i++) {
+		struct clk_hw *parent = clk_hw_get_parent_by_index(hw, i);
+		unsigned long parent_rate = clk_hw_get_rate(parent);
+
+		max_div = BIT(md->hid_width) - 1;
+		for (div = 1; div < max_div; div++) {
+			parent_rate = mult_frac(rate, div, 2);
+			parent_rate = clk_hw_round_rate(parent, parent_rate);
+			actual_rate = mult_frac(parent_rate, 2, div);
+
+			if (is_better_rate(rate, best_rate, actual_rate)) {
+				best_rate = actual_rate;
+				best_src = md->parent_map[i].cfg;
+				best_div = div - 1;
+			}
+
+			if (actual_rate < rate || best_rate <= rate)
+				break;
+		}
+	}
+
+	ret = __mux_div_set_src_div(md, best_src, best_div);
+	if (!ret) {
+		md->div = best_div;
+		md->src = best_src;
+	}
+
+	return ret;
+}
+
+static u8 mux_div_get_parent(struct clk_hw *hw)
+{
+	struct clk_regmap_mux_div *md = to_clk_regmap_mux_div(hw);
+	const char *name = clk_hw_get_name(hw);
+	u32 i, div, src = 0;
+
+	__mux_div_get_src_div(md, &src, &div);
+
+	for (i = 0; i < clk_hw_get_num_parents(hw); i++)
+		if (src == md->parent_map[i].cfg)
+			return i;
+
+	pr_err("%s: Can't find parent with src %d\n", name, src);
+	return 0;
+}
+
+static int mux_div_set_parent(struct clk_hw *hw, u8 index)
+{
+	struct clk_regmap_mux_div *md = to_clk_regmap_mux_div(hw);
+
+	return __mux_div_set_src_div(md, md->parent_map[index].cfg, md->div);
+}
+
+static int mux_div_set_rate(struct clk_hw *hw,
+			    unsigned long rate, unsigned long prate)
+{
+	struct clk_regmap_mux_div *md = to_clk_regmap_mux_div(hw);
+
+	return __mux_div_set_rate_and_parent(hw, rate, prate, md->src);
+}
+
+static int mux_div_set_rate_and_parent(struct clk_hw *hw,  unsigned long rate,
+				       unsigned long prate, u8 index)
+{
+	struct clk_regmap_mux_div *md = to_clk_regmap_mux_div(hw);
+
+	return __mux_div_set_rate_and_parent(hw, rate, prate,
+					     md->parent_map[index].cfg);
+}
+
+static unsigned long mux_div_recalc_rate(struct clk_hw *hw, unsigned long prate)
+{
+	struct clk_regmap_mux_div *md = to_clk_regmap_mux_div(hw);
+	u32 div, src;
+	int i, num_parents = clk_hw_get_num_parents(hw);
+	const char *name = clk_hw_get_name(hw);
+
+	__mux_div_get_src_div(md, &src, &div);
+	for (i = 0; i < num_parents; i++)
+		if (src == md->parent_map[i].cfg) {
+			struct clk_hw *p = clk_hw_get_parent_by_index(hw, i);
+			unsigned long parent_rate = clk_hw_get_rate(p);
+
+			return mult_frac(parent_rate, 2, div + 1);
+		}
+
+	pr_err("%s: Can't find parent %d\n", name, src);
+	return 0;
+}
+
+const struct clk_ops clk_regmap_mux_div_ops = {
+	.get_parent = mux_div_get_parent,
+	.set_parent = mux_div_set_parent,
+	.set_rate = mux_div_set_rate,
+	.set_rate_and_parent = mux_div_set_rate_and_parent,
+	.determine_rate = mux_div_determine_rate,
+	.recalc_rate = mux_div_recalc_rate,
+};
diff --git a/drivers/clk/qcom/clk-regmap-mux-div.h b/drivers/clk/qcom/clk-regmap-mux-div.h
new file mode 100644
index 000000000000..a35da978d6c9
--- /dev/null
+++ b/drivers/clk/qcom/clk-regmap-mux-div.h
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2017, Linaro Limited
+ * Author: Georgi Djakov <georgi.djakov@linaro.org>
+ */
+
+#ifndef __QCOM_CLK_REGMAP_MUX_DIV_H__
+#define __QCOM_CLK_REGMAP_MUX_DIV_H__
+
+#include <linux/clk-provider.h>
+#include "clk-rcg.h"
+#include "clk-regmap.h"
+
+/**
+ * struct mux_div_clk - combined mux/divider clock
+ * @reg_offset: offset of the mux/divider register
+ * @hid_width:	number of bits in half integer divider
+ * @hid_shift:	lowest bit of hid value field
+ * @src_width:	number of bits in source select
+ * @src_shift:	lowest bit of source select field
+ * @div:	the divider raw configuration value
+ * @src:	the mux index which will be used if the clock is enabled
+ * @parent_map:	pointer to parent_map struct
+ * @clkr:	handle between common and hardware-specific interfaces
+ * @pclk:	the input PLL clock
+ * @clk_nb:	clock notifier for rate changes of the input PLL
+ */
+
+struct clk_regmap_mux_div {
+	u32				reg_offset;
+	u32				hid_width;
+	u32				hid_shift;
+	u32				src_width;
+	u32				src_shift;
+	u32				div;
+	u32				src;
+	const struct parent_map		*parent_map;
+	struct clk_regmap		clkr;
+	struct clk			*pclk;
+	struct notifier_block		clk_nb;
+};
+
+extern const struct clk_ops clk_regmap_mux_div_ops;
+int __mux_div_set_src_div(struct clk_regmap_mux_div *md, u32 src, u32 div);
+
+#endif

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

* [PATCH v11 5/6] dt-bindings: mailbox: qcom: Document the APCS clock binding
  2017-12-05 15:46 [PATCH v11 0/6] Add support for Qualcomm A53 CPU clock Georgi Djakov
                   ` (3 preceding siblings ...)
  2017-12-05 15:46 ` [PATCH v11 4/6] clk: qcom: Add regmap mux-div clocks support Georgi Djakov
@ 2017-12-05 15:47 ` Georgi Djakov
       [not found] ` <20171205154701.27730-1-georgi.djakov-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 27+ messages in thread
From: Georgi Djakov @ 2017-12-05 15:47 UTC (permalink / raw)
  To: sboyd, jassisinghbrar, bjorn.andersson
  Cc: mturquette, robh, linux-clk, linux-kernel, linux-arm-msm,
	devicetree, georgi.djakov

Update the binding documentation for APCS to mention that the APCS
hardware block also expose a clock controller functionality.

The APCS clock controller is a mux and half-integer divider. It has the
main CPU PLL as an input and provides the clock for the application CPU.

Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 .../bindings/mailbox/qcom,apcs-kpss-global.txt         | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt
index fb961c310f44..16964f0c1773 100644
--- a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt
+++ b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt
@@ -15,12 +15,21 @@ platforms.
 	Usage: required
 	Value type: <prop-encoded-array>
 	Definition: must specify the base address and size of the global block
+- clocks:
+	Usage: required if #clocks-cells property is present
+	Value type: <phandle>
+	Definition: phandle to the input PLL, which feeds the APCS mux/divider
 
 - #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
+
 
 = EXAMPLE
 The following example describes the APCS HMSS found in MSM8996 and part of the
@@ -44,3 +53,12 @@ GLINK RPM referencing the "rpm_hlos" doorbell therein.
 		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>;
+	};

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

* [PATCH v11 6/6] clk: qcom: Add APCS clock controller support
       [not found] ` <20171205154701.27730-1-georgi.djakov-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
@ 2017-12-05 15:47   ` Georgi Djakov
       [not found]     ` <20171205154701.27730-7-georgi.djakov-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
  2017-12-06  5:51   ` [PATCH v11 0/6] Add support for Qualcomm A53 CPU clock Amit Kucheria
  1 sibling, 1 reply; 27+ messages in thread
From: Georgi Djakov @ 2017-12-05 15:47 UTC (permalink / raw)
  To: sboyd-sgV2jX0FEOL9JmXXK+q4OQ,
	jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w,
	bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A
  Cc: mturquette-rdvid1DuHRBWk0Htik3J/w, robh-DgEjT+Ai2ygdnm+yROfE0A,
	linux-clk-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	georgi.djakov-QSEj5FYQhm4dnm+yROfE0A

Add a driver for the APCS clock controller. It is part of the APCS
hardware block, which among other things implements also a combined
mux and half integer divider functionality. It can choose between a
fixed-rate clock or the dedicated APCS (A53) PLL. The source and the
divider can be set both at the same time.

This is required for enabling CPU frequency scaling on MSM8916-based
platforms.

Signed-off-by: Georgi Djakov <georgi.djakov-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/clk/qcom/Kconfig        |  11 +++
 drivers/clk/qcom/Makefile       |   1 +
 drivers/clk/qcom/apcs-msm8916.c | 149 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 161 insertions(+)
 create mode 100644 drivers/clk/qcom/apcs-msm8916.c

diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index 81ac7b9378fe..255023b439c9 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -22,6 +22,17 @@ config QCOM_A53PLL
 	  Say Y if you want to support higher CPU frequencies on MSM8916
 	  devices.
 
+config QCOM_CLK_APCS_MSM8916
+	bool "MSM8916 APCS Clock Controller"
+	depends on COMMON_CLK_QCOM
+	depends on QCOM_APCS_IPC
+	default ARCH_QCOM
+	help
+	  Support for the APCS Clock Controller on msm8916 devices. The
+	  APCS is managing the mux and divider which feeds the CPUs.
+	  Say Y if you want to support CPU frequency scaling on devices
+	  such as msm8916.
+
 config QCOM_CLK_RPM
 	tristate "RPM based Clock Controller"
 	depends on COMMON_CLK_QCOM && MFD_QCOM_RPM
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index 7c51d877f967..0408cebf38d4 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -34,5 +34,6 @@ obj-$(CONFIG_MSM_MMCC_8960) += mmcc-msm8960.o
 obj-$(CONFIG_MSM_MMCC_8974) += mmcc-msm8974.o
 obj-$(CONFIG_MSM_MMCC_8996) += mmcc-msm8996.o
 obj-$(CONFIG_QCOM_A53PLL) += a53-pll.o
+obj-$(CONFIG_QCOM_CLK_APCS_MSM8916) += apcs-msm8916.o
 obj-$(CONFIG_QCOM_CLK_RPM) += clk-rpm.o
 obj-$(CONFIG_QCOM_CLK_SMD_RPM) += clk-smd-rpm.o
diff --git a/drivers/clk/qcom/apcs-msm8916.c b/drivers/clk/qcom/apcs-msm8916.c
new file mode 100644
index 000000000000..832172c2fc8b
--- /dev/null
+++ b/drivers/clk/qcom/apcs-msm8916.c
@@ -0,0 +1,149 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Qualcomm APCS clock controller driver
+ *
+ * Copyright (c) 2017, Linaro Limited
+ * Author: Georgi Djakov <georgi.djakov-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/kernel.h>
+#include <linux/mailbox_controller.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include "clk-regmap.h"
+#include "clk-regmap-mux-div.h"
+
+enum {
+	P_GPLL0,
+	P_A53PLL,
+};
+
+static const struct parent_map gpll0_a53cc_map[] = {
+	{ P_GPLL0, 4 },
+	{ P_A53PLL, 5 },
+};
+
+static const char * const gpll0_a53cc[] = {
+	"gpll0_vote",
+	"a53pll",
+};
+
+/*
+ * We use the notifier function for switching to a temporary safe configuration
+ * (mux and divider), while the A53 PLL is reconfigured.
+ */
+static int a53cc_notifier_cb(struct notifier_block *nb, unsigned long event,
+			     void *data)
+{
+	int ret = 0;
+	struct clk_regmap_mux_div *md = container_of(nb,
+						     struct clk_regmap_mux_div,
+						     clk_nb);
+	if (event == PRE_RATE_CHANGE)
+		/* set the mux and divider to safe frequency (400mhz) */
+		ret = __mux_div_set_src_div(md, 4, 3);
+
+	return notifier_from_errno(ret);
+}
+
+static int qcom_apcs_msm8916_clk_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device *parent = dev->parent;
+	struct clk_regmap_mux_div *a53cc;
+	struct regmap *regmap;
+	struct clk_init_data init = { };
+	int ret;
+
+	regmap = dev_get_regmap(parent, NULL);
+	if (IS_ERR(regmap)) {
+		ret = PTR_ERR(regmap);
+		dev_err(dev, "failed to get regmap: %d\n", ret);
+		return ret;
+	}
+
+	a53cc = devm_kzalloc(dev, sizeof(*a53cc), GFP_KERNEL);
+	if (!a53cc)
+		return -ENOMEM;
+
+	init.name = "a53mux";
+	init.parent_names = gpll0_a53cc;
+	init.num_parents = ARRAY_SIZE(gpll0_a53cc);
+	init.ops = &clk_regmap_mux_div_ops;
+	init.flags = CLK_SET_RATE_PARENT;
+
+	a53cc->clkr.hw.init = &init;
+	a53cc->clkr.regmap = regmap;
+	a53cc->reg_offset = 0x50;
+	a53cc->hid_width = 5;
+	a53cc->hid_shift = 0;
+	a53cc->src_width = 3;
+	a53cc->src_shift = 8;
+	a53cc->parent_map = gpll0_a53cc_map;
+
+	a53cc->pclk = devm_clk_get(parent, NULL);
+	if (IS_ERR(a53cc->pclk)) {
+		ret = PTR_ERR(a53cc->pclk);
+		dev_err(dev, "failed to get clk: %d\n", ret);
+		return ret;
+	}
+
+	a53cc->clk_nb.notifier_call = a53cc_notifier_cb;
+	ret = clk_notifier_register(a53cc->pclk, &a53cc->clk_nb);
+	if (ret) {
+		dev_err(dev, "failed to register clock notifier: %d\n", ret);
+		return ret;
+	}
+
+	ret = devm_clk_register_regmap(dev, &a53cc->clkr);
+	if (ret) {
+		dev_err(dev, "failed to register regmap clock: %d\n", ret);
+		goto err;
+	}
+
+	ret = of_clk_add_hw_provider(parent->of_node, of_clk_hw_simple_get,
+				     &a53cc->clkr.hw);
+	if (ret) {
+		dev_err(dev, "failed to add clock provider: %d\n", ret);
+		goto err;
+	}
+
+	platform_set_drvdata(pdev, a53cc);
+
+	return 0;
+
+err:
+	clk_notifier_unregister(a53cc->pclk, &a53cc->clk_nb);
+	return ret;
+}
+
+static int qcom_apcs_msm8916_clk_remove(struct platform_device *pdev)
+{
+	struct clk_regmap_mux_div *a53cc = platform_get_drvdata(pdev);
+	struct device *parent = pdev->dev.parent;
+
+	clk_notifier_unregister(a53cc->pclk, &a53cc->clk_nb);
+	of_clk_del_provider(parent->of_node);
+
+	return 0;
+}
+
+static struct platform_driver qcom_apcs_msm8916_clk_driver = {
+	.probe = qcom_apcs_msm8916_clk_probe,
+	.remove = qcom_apcs_msm8916_clk_remove,
+	.driver = {
+		.name = "qcom-apcs-msm8916-clk",
+	},
+};
+module_platform_driver(qcom_apcs_msm8916_clk_driver);
+
+MODULE_AUTHOR("Georgi Djakov <georgi.djakov-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Qualcomm MSM8916 APCS clock driver");
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v11 6/6] clk: qcom: Add APCS clock controller support
       [not found]     ` <20171205154701.27730-7-georgi.djakov-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
@ 2017-12-05 17:21       ` Bjorn Andersson
  2017-12-29  0:00       ` Stephen Boyd
  1 sibling, 0 replies; 27+ messages in thread
From: Bjorn Andersson @ 2017-12-05 17:21 UTC (permalink / raw)
  To: Georgi Djakov
  Cc: sboyd-sgV2jX0FEOL9JmXXK+q4OQ,
	jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w,
	mturquette-rdvid1DuHRBWk0Htik3J/w, robh-DgEjT+Ai2ygdnm+yROfE0A,
	linux-clk-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

On Tue 05 Dec 07:47 PST 2017, Georgi Djakov wrote:

> Add a driver for the APCS clock controller. It is part of the APCS
> hardware block, which among other things implements also a combined
> mux and half integer divider functionality. It can choose between a
> fixed-rate clock or the dedicated APCS (A53) PLL. The source and the
> divider can be set both at the same time.
> 
> This is required for enabling CPU frequency scaling on MSM8916-based
> platforms.
> 

Acked-by: Bjorn Andersson <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Regards,
Bjorn

> Signed-off-by: Georgi Djakov <georgi.djakov-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  drivers/clk/qcom/Kconfig        |  11 +++
>  drivers/clk/qcom/Makefile       |   1 +
>  drivers/clk/qcom/apcs-msm8916.c | 149 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 161 insertions(+)
>  create mode 100644 drivers/clk/qcom/apcs-msm8916.c
> 
> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> index 81ac7b9378fe..255023b439c9 100644
> --- a/drivers/clk/qcom/Kconfig
> +++ b/drivers/clk/qcom/Kconfig
> @@ -22,6 +22,17 @@ config QCOM_A53PLL
>  	  Say Y if you want to support higher CPU frequencies on MSM8916
>  	  devices.
>  
> +config QCOM_CLK_APCS_MSM8916
> +	bool "MSM8916 APCS Clock Controller"
> +	depends on COMMON_CLK_QCOM
> +	depends on QCOM_APCS_IPC
> +	default ARCH_QCOM
> +	help
> +	  Support for the APCS Clock Controller on msm8916 devices. The
> +	  APCS is managing the mux and divider which feeds the CPUs.
> +	  Say Y if you want to support CPU frequency scaling on devices
> +	  such as msm8916.
> +
>  config QCOM_CLK_RPM
>  	tristate "RPM based Clock Controller"
>  	depends on COMMON_CLK_QCOM && MFD_QCOM_RPM
> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
> index 7c51d877f967..0408cebf38d4 100644
> --- a/drivers/clk/qcom/Makefile
> +++ b/drivers/clk/qcom/Makefile
> @@ -34,5 +34,6 @@ obj-$(CONFIG_MSM_MMCC_8960) += mmcc-msm8960.o
>  obj-$(CONFIG_MSM_MMCC_8974) += mmcc-msm8974.o
>  obj-$(CONFIG_MSM_MMCC_8996) += mmcc-msm8996.o
>  obj-$(CONFIG_QCOM_A53PLL) += a53-pll.o
> +obj-$(CONFIG_QCOM_CLK_APCS_MSM8916) += apcs-msm8916.o
>  obj-$(CONFIG_QCOM_CLK_RPM) += clk-rpm.o
>  obj-$(CONFIG_QCOM_CLK_SMD_RPM) += clk-smd-rpm.o
> diff --git a/drivers/clk/qcom/apcs-msm8916.c b/drivers/clk/qcom/apcs-msm8916.c
> new file mode 100644
> index 000000000000..832172c2fc8b
> --- /dev/null
> +++ b/drivers/clk/qcom/apcs-msm8916.c
> @@ -0,0 +1,149 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Qualcomm APCS clock controller driver
> + *
> + * Copyright (c) 2017, Linaro Limited
> + * Author: Georgi Djakov <georgi.djakov-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/kernel.h>
> +#include <linux/mailbox_controller.h>
> +#include <linux/module.h>
> +#include <linux/io.h>
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include "clk-regmap.h"
> +#include "clk-regmap-mux-div.h"
> +
> +enum {
> +	P_GPLL0,
> +	P_A53PLL,
> +};
> +
> +static const struct parent_map gpll0_a53cc_map[] = {
> +	{ P_GPLL0, 4 },
> +	{ P_A53PLL, 5 },
> +};
> +
> +static const char * const gpll0_a53cc[] = {
> +	"gpll0_vote",
> +	"a53pll",
> +};
> +
> +/*
> + * We use the notifier function for switching to a temporary safe configuration
> + * (mux and divider), while the A53 PLL is reconfigured.
> + */
> +static int a53cc_notifier_cb(struct notifier_block *nb, unsigned long event,
> +			     void *data)
> +{
> +	int ret = 0;
> +	struct clk_regmap_mux_div *md = container_of(nb,
> +						     struct clk_regmap_mux_div,
> +						     clk_nb);
> +	if (event == PRE_RATE_CHANGE)
> +		/* set the mux and divider to safe frequency (400mhz) */
> +		ret = __mux_div_set_src_div(md, 4, 3);
> +
> +	return notifier_from_errno(ret);
> +}
> +
> +static int qcom_apcs_msm8916_clk_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device *parent = dev->parent;
> +	struct clk_regmap_mux_div *a53cc;
> +	struct regmap *regmap;
> +	struct clk_init_data init = { };
> +	int ret;
> +
> +	regmap = dev_get_regmap(parent, NULL);
> +	if (IS_ERR(regmap)) {
> +		ret = PTR_ERR(regmap);
> +		dev_err(dev, "failed to get regmap: %d\n", ret);
> +		return ret;
> +	}
> +
> +	a53cc = devm_kzalloc(dev, sizeof(*a53cc), GFP_KERNEL);
> +	if (!a53cc)
> +		return -ENOMEM;
> +
> +	init.name = "a53mux";
> +	init.parent_names = gpll0_a53cc;
> +	init.num_parents = ARRAY_SIZE(gpll0_a53cc);
> +	init.ops = &clk_regmap_mux_div_ops;
> +	init.flags = CLK_SET_RATE_PARENT;
> +
> +	a53cc->clkr.hw.init = &init;
> +	a53cc->clkr.regmap = regmap;
> +	a53cc->reg_offset = 0x50;
> +	a53cc->hid_width = 5;
> +	a53cc->hid_shift = 0;
> +	a53cc->src_width = 3;
> +	a53cc->src_shift = 8;
> +	a53cc->parent_map = gpll0_a53cc_map;
> +
> +	a53cc->pclk = devm_clk_get(parent, NULL);
> +	if (IS_ERR(a53cc->pclk)) {
> +		ret = PTR_ERR(a53cc->pclk);
> +		dev_err(dev, "failed to get clk: %d\n", ret);
> +		return ret;
> +	}
> +
> +	a53cc->clk_nb.notifier_call = a53cc_notifier_cb;
> +	ret = clk_notifier_register(a53cc->pclk, &a53cc->clk_nb);
> +	if (ret) {
> +		dev_err(dev, "failed to register clock notifier: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = devm_clk_register_regmap(dev, &a53cc->clkr);
> +	if (ret) {
> +		dev_err(dev, "failed to register regmap clock: %d\n", ret);
> +		goto err;
> +	}
> +
> +	ret = of_clk_add_hw_provider(parent->of_node, of_clk_hw_simple_get,
> +				     &a53cc->clkr.hw);
> +	if (ret) {
> +		dev_err(dev, "failed to add clock provider: %d\n", ret);
> +		goto err;
> +	}
> +
> +	platform_set_drvdata(pdev, a53cc);
> +
> +	return 0;
> +
> +err:
> +	clk_notifier_unregister(a53cc->pclk, &a53cc->clk_nb);
> +	return ret;
> +}
> +
> +static int qcom_apcs_msm8916_clk_remove(struct platform_device *pdev)
> +{
> +	struct clk_regmap_mux_div *a53cc = platform_get_drvdata(pdev);
> +	struct device *parent = pdev->dev.parent;
> +
> +	clk_notifier_unregister(a53cc->pclk, &a53cc->clk_nb);
> +	of_clk_del_provider(parent->of_node);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver qcom_apcs_msm8916_clk_driver = {
> +	.probe = qcom_apcs_msm8916_clk_probe,
> +	.remove = qcom_apcs_msm8916_clk_remove,
> +	.driver = {
> +		.name = "qcom-apcs-msm8916-clk",
> +	},
> +};
> +module_platform_driver(qcom_apcs_msm8916_clk_driver);
> +
> +MODULE_AUTHOR("Georgi Djakov <georgi.djakov-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Qualcomm MSM8916 APCS clock driver");
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v11 0/6] Add support for Qualcomm A53 CPU clock
       [not found] ` <20171205154701.27730-1-georgi.djakov-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
  2017-12-05 15:47   ` [PATCH v11 6/6] clk: qcom: Add APCS clock controller support Georgi Djakov
@ 2017-12-06  5:51   ` Amit Kucheria
  2017-12-07 16:00     ` Georgi Djakov
  1 sibling, 1 reply; 27+ messages in thread
From: Amit Kucheria @ 2017-12-06  5:51 UTC (permalink / raw)
  To: Georgi Djakov
  Cc: Stephen Boyd, jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w,
	bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A, Michael Turquette,
	Rob Herring, linux-clk-u79uwXL29TY76Z2rM5mHXA, LKML,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

On Tue, Dec 5, 2017 at 9:16 PM, Georgi Djakov <georgi.djakov-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> This patchset adds support for the A53 CPU clock on MSM8916 platforms
> and allows scaling of the CPU frequency on msm8916 based platforms.

Though it currently needs some additional patches (that'll follow
soon), FWIW, Tested-by: Amit Kucheria <amit.kucheria-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

> Changes since v10 (https://lkml.org/lkml/2017/12/1/577)
> * Addressed Bjorn's comments on APCS clock driver.
> * Picked Acks from Rob and Bjorn.
>
> Changes since v9 (https://lkml.org/lkml/2017/9/21/511)
> * Added the clock properties to the APCS DT node, instead of adding a subnode
> and also replaced patch "mailbox: qcom: Populate APCS child platform devices"
> with "mailbox: qcom: Create APCS child device for clock controller".
> * Dropped patch "mailbox: qcom: Move the apcs struct into a separate header",
> and use dev_get_regmap(dev->parent) in the child driver.
> * Addressed Bjorn's comments on a53-pll and apcs-clk drivers.
> * Added SPDX copyright identifiers.
>
> Changes since v8 (https://lkml.org/lkml/2017/6/23/476)
>  * Converted APCS mailbox driver to use regmap and to populate child
>  platform devices that will handle the rest of the functionality
>  provided by APCS block.
>  * Picked Rob's Ack for the PLL binding.
>  * Changed the APCS binding and put it into a separate patch.
>  * Addressed review comments.
>  * Minor changes.
>
> Changes since v7 (https://lkml.org/lkml/2016/10/31/296)
>  * Add the APCS clock controller to the APCS driver to expose both the
>  mailbox and clock controller functionality as discussed earlier:
>  https://lkml.org/lkml/2016/11/14/860
>  * Changed the a53pll compatible string as suggested by Rob.
>
> Changes since v6 (https://lkml.org/lkml/2016/9/7/347)
>  * Addressed various comments from Stephen Boyd
>
> Changes since v5 (https://lkml.org/lkml/2016/2/1/407)
>  * Rebase to clk-next and update according to the recent API changes.
>
> Changes since v4 (https://lkml.org/lkml/2015/12/14/367)
>  * Convert to builtin drivers as now __clk_lookup() is used
>
> Changes since v3 (https://lkml.org/lkml/2015/8/12/585)
>  * Split driver into two parts - and separate A53 PLL and
>    A53 clock controller drivers.
>  * Drop the safe switch hook patch. Add a clock notifier in
>    the clock provider to handle switching via safe mux and
>    divider configuration.
>
> Changes since v2 (https://lkml.org/lkml/2015/7/24/526)
>  * Drop gpll0_vote patch.
>  * Switch to the new clk_hw_* APIs.
>  * Rebase to the current clk-next.
>
> Changes since v1 (https://lkml.org/lkml/2015/6/12/193)
>  * Drop SR2 PLL patch, as it is already applied.
>  * Add gpll0_vote rate propagation patch.
>  * Update/rebase patches to the current clk-next.
>
> Georgi Djakov (6):
>   mailbox: qcom: Convert APCS IPC driver to use regmap
>   mailbox: qcom: Create APCS child device for clock controller
>   clk: qcom: Add A53 PLL support
>   clk: qcom: Add regmap mux-div clocks support
>   dt-bindings: mailbox: qcom: Document the APCS clock binding
>   clk: qcom: Add APCS clock controller support
>
>  .../devicetree/bindings/clock/qcom,a53pll.txt      |  22 ++
>  .../bindings/mailbox/qcom,apcs-kpss-global.txt     |  18 ++
>  drivers/clk/qcom/Kconfig                           |  21 ++
>  drivers/clk/qcom/Makefile                          |   3 +
>  drivers/clk/qcom/a53-pll.c                         | 110 ++++++++++
>  drivers/clk/qcom/apcs-msm8916.c                    | 149 ++++++++++++++
>  drivers/clk/qcom/clk-regmap-mux-div.c              | 229 +++++++++++++++++++++
>  drivers/clk/qcom/clk-regmap-mux-div.h              |  46 +++++
>  drivers/mailbox/qcom-apcs-ipc-mailbox.c            |  35 +++-
>  9 files changed, 628 insertions(+), 5 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/clock/qcom,a53pll.txt
>  create mode 100644 drivers/clk/qcom/a53-pll.c
>  create mode 100644 drivers/clk/qcom/apcs-msm8916.c
>  create mode 100644 drivers/clk/qcom/clk-regmap-mux-div.c
>  create mode 100644 drivers/clk/qcom/clk-regmap-mux-div.h
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v11 0/6] Add support for Qualcomm A53 CPU clock
  2017-12-05 15:46 [PATCH v11 0/6] Add support for Qualcomm A53 CPU clock Georgi Djakov
                   ` (5 preceding siblings ...)
       [not found] ` <20171205154701.27730-1-georgi.djakov-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
@ 2017-12-06 21:08 ` Rob Herring
  2017-12-07 16:02   ` Georgi Djakov
  2017-12-22  0:49 ` Stephen Boyd
  2018-01-26 14:30 ` Georgi Djakov
  8 siblings, 1 reply; 27+ messages in thread
From: Rob Herring @ 2017-12-06 21:08 UTC (permalink / raw)
  To: Georgi Djakov
  Cc: sboyd, jassisinghbrar, bjorn.andersson, mturquette, linux-clk,
	linux-kernel, linux-arm-msm, devicetree

On Tue, Dec 05, 2017 at 05:46:55PM +0200, Georgi Djakov wrote:
> This patchset adds support for the A53 CPU clock on MSM8916 platforms
> and allows scaling of the CPU frequency on msm8916 based platforms.
> 
> Changes since v10 (https://lkml.org/lkml/2017/12/1/577)
> * Addressed Bjorn's comments on APCS clock driver.
> * Picked Acks from Rob and Bjorn.

Errr, really?

Rob

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

* Re: [PATCH v11 0/6] Add support for Qualcomm A53 CPU clock
  2017-12-06  5:51   ` [PATCH v11 0/6] Add support for Qualcomm A53 CPU clock Amit Kucheria
@ 2017-12-07 16:00     ` Georgi Djakov
  0 siblings, 0 replies; 27+ messages in thread
From: Georgi Djakov @ 2017-12-07 16:00 UTC (permalink / raw)
  To: Amit Kucheria
  Cc: Stephen Boyd, jassisinghbrar, bjorn.andersson, Michael Turquette,
	Rob Herring, linux-clk, LKML, linux-arm-msm, devicetree

Hi Amit,

On 12/06/2017 07:51 AM, Amit Kucheria wrote:
> On Tue, Dec 5, 2017 at 9:16 PM, Georgi Djakov <georgi.djakov@linaro.org> wrote:
>> This patchset adds support for the A53 CPU clock on MSM8916 platforms
>> and allows scaling of the CPU frequency on msm8916 based platforms.
> 
> Though it currently needs some additional patches (that'll follow
> soon), FWIW, Tested-by: Amit Kucheria <amit.kucheria@linaro.org>

Thanks for testing! I will submit the platform specific dts changes as
separate follow-up patches when this is merged.

BR,
Georgi

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

* Re: [PATCH v11 0/6] Add support for Qualcomm A53 CPU clock
  2017-12-06 21:08 ` Rob Herring
@ 2017-12-07 16:02   ` Georgi Djakov
  0 siblings, 0 replies; 27+ messages in thread
From: Georgi Djakov @ 2017-12-07 16:02 UTC (permalink / raw)
  To: Rob Herring
  Cc: sboyd, jassisinghbrar, bjorn.andersson, mturquette, linux-clk,
	linux-kernel, linux-arm-msm, devicetree

On 12/06/2017 11:08 PM, Rob Herring wrote:
> On Tue, Dec 05, 2017 at 05:46:55PM +0200, Georgi Djakov wrote:
>> This patchset adds support for the A53 CPU clock on MSM8916 platforms
>> and allows scaling of the CPU frequency on msm8916 based platforms.
>>
>> Changes since v10 (https://lkml.org/lkml/2017/12/1/577)
>> * Addressed Bjorn's comments on APCS clock driver.
>> * Picked Acks from Rob and Bjorn.
> 
> Errr, really?

Oops sorry, it was a Reviewed-by for the mailbox binding change and not
an Ack. Thank you for reviewing the constantly changing binding in this
patchset. I hope that it will not change anymore.

BR,
Georgi

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

* Re: [PATCH v11 0/6] Add support for Qualcomm A53 CPU clock
  2017-12-05 15:46 [PATCH v11 0/6] Add support for Qualcomm A53 CPU clock Georgi Djakov
                   ` (6 preceding siblings ...)
  2017-12-06 21:08 ` Rob Herring
@ 2017-12-22  0:49 ` Stephen Boyd
  2017-12-22 14:52   ` Georgi Djakov
  2018-01-26 14:30 ` Georgi Djakov
  8 siblings, 1 reply; 27+ messages in thread
From: Stephen Boyd @ 2017-12-22  0:49 UTC (permalink / raw)
  To: Georgi Djakov
  Cc: jassisinghbrar, bjorn.andersson, mturquette, robh, linux-clk,
	linux-kernel, linux-arm-msm, devicetree

On 12/05, Georgi Djakov wrote:
> This patchset adds support for the A53 CPU clock on MSM8916 platforms
> and allows scaling of the CPU frequency on msm8916 based platforms.

Ok. I will apply just the clk ones? Patches 3,4, and 6?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* Re: [PATCH v11 0/6] Add support for Qualcomm A53 CPU clock
  2017-12-22  0:49 ` Stephen Boyd
@ 2017-12-22 14:52   ` Georgi Djakov
  2017-12-28 23:53     ` Stephen Boyd
  0 siblings, 1 reply; 27+ messages in thread
From: Georgi Djakov @ 2017-12-22 14:52 UTC (permalink / raw)
  To: Stephen Boyd, jassisinghbrar
  Cc: bjorn.andersson, mturquette, robh, linux-clk, linux-kernel,
	linux-arm-msm, devicetree

On 22.12.17 г. 2:49, Stephen Boyd wrote:
> On 12/05, Georgi Djakov wrote:
>> This patchset adds support for the A53 CPU clock on MSM8916 platforms
>> and allows scaling of the CPU frequency on msm8916 based platforms.
> 
> Ok. I will apply just the clk ones? Patches 3,4, and 6?
> 

Yes, i think this would be best. The rest should go through the mailbox tree.
Jassi, are you ok with this?

Thanks,
Georgi

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

* Re: [PATCH v11 2/6] mailbox: qcom: Create APCS child device for clock controller
  2017-12-05 15:46 ` [PATCH v11 2/6] mailbox: qcom: Create APCS child device for clock controller Georgi Djakov
@ 2017-12-23  4:57   ` Jassi Brar
       [not found]     ` <CABb+yY146ho0gtbUzDv-Z_xESkU2y=UYfBA6i+522+m58LkABQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 27+ messages in thread
From: Jassi Brar @ 2017-12-23  4:57 UTC (permalink / raw)
  To: Georgi Djakov
  Cc: Stephen Boyd, Bjorn Andersson, Michael Turquette, Rob Herring,
	linux-clk, Linux Kernel Mailing List, linux-arm-msm,
	Devicetree List

On Tue, Dec 5, 2017 at 9:16 PM, Georgi Djakov <georgi.djakov@linaro.org> wrote:
> There is a clock controller functionality provided by the APCS hardware
> block of msm8916 devices. The device-tree would represent an APCS node
> with both mailbox and clock provider properties.
>
The spec might depict a 'clock' box and 'mailbox' box inside the
bigger APCS box. However, from the code I see in this patchset, they
are orthogonal and can & should be represented as independent DT
nodes.
Patch-1 is ok, though.

Thanks

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

* Re: [PATCH v11 2/6] mailbox: qcom: Create APCS child device for clock controller
       [not found]     ` <CABb+yY146ho0gtbUzDv-Z_xESkU2y=UYfBA6i+522+m58LkABQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2017-12-24  5:06       ` Bjorn Andersson
  2017-12-29  6:14         ` Jassi Brar
  0 siblings, 1 reply; 27+ messages in thread
From: Bjorn Andersson @ 2017-12-24  5:06 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Georgi Djakov, Stephen Boyd, Michael Turquette, Rob Herring,
	linux-clk-u79uwXL29TY76Z2rM5mHXA, Linux Kernel Mailing List,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Devicetree List

On Fri 22 Dec 20:57 PST 2017, Jassi Brar wrote:

> On Tue, Dec 5, 2017 at 9:16 PM, Georgi Djakov <georgi.djakov-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > There is a clock controller functionality provided by the APCS hardware
> > block of msm8916 devices. The device-tree would represent an APCS node
> > with both mailbox and clock provider properties.
> >
> The spec might depict a 'clock' box and 'mailbox' box inside the
> bigger APCS box. However, from the code I see in this patchset, they
> are orthogonal and can & should be represented as independent DT
> nodes.

The APCS consists of a number of different hardware blocks, one of them
being the "APCS global" block, which is what this node and drivers
relate to. On 8916 this contains both the IPC register and clock
control. But it's still just one block according to the hardware
specification.

As such DT should describe the one hardware block by one node IMHO. The
fact that we in Linux split this into two different drivers is an
unrelated implementation detail.

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v11 0/6] Add support for Qualcomm A53 CPU clock
  2017-12-22 14:52   ` Georgi Djakov
@ 2017-12-28 23:53     ` Stephen Boyd
  0 siblings, 0 replies; 27+ messages in thread
From: Stephen Boyd @ 2017-12-28 23:53 UTC (permalink / raw)
  To: Georgi Djakov
  Cc: jassisinghbrar, bjorn.andersson, mturquette, robh, linux-clk,
	linux-kernel, linux-arm-msm, devicetree

On 12/22, Georgi Djakov wrote:
> On 22.12.17 г. 2:49, Stephen Boyd wrote:
> > On 12/05, Georgi Djakov wrote:
> >> This patchset adds support for the A53 CPU clock on MSM8916 platforms
> >> and allows scaling of the CPU frequency on msm8916 based platforms.
> > 
> > Ok. I will apply just the clk ones? Patches 3,4, and 6?
> > 
> 
> Yes, i think this would be best. The rest should go through the mailbox tree.
> 

Ok. I made some changes. I'll comment one patch too. But I
applied them to clk-next.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* Re: [PATCH v11 6/6] clk: qcom: Add APCS clock controller support
       [not found]     ` <20171205154701.27730-7-georgi.djakov-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
  2017-12-05 17:21       ` Bjorn Andersson
@ 2017-12-29  0:00       ` Stephen Boyd
  1 sibling, 0 replies; 27+ messages in thread
From: Stephen Boyd @ 2017-12-29  0:00 UTC (permalink / raw)
  To: Georgi Djakov
  Cc: jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w,
	bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A,
	mturquette-rdvid1DuHRBWk0Htik3J/w, robh-DgEjT+Ai2ygdnm+yROfE0A,
	linux-clk-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

On 12/05, Georgi Djakov wrote:
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include "clk-regmap.h"
> +#include "clk-regmap-mux-div.h"
> +
> +enum {
> +	P_GPLL0,
> +	P_A53PLL,
> +};

This is always 0, 1.

> +
> +static const struct parent_map gpll0_a53cc_map[] = {
> +	{ P_GPLL0, 4 },
> +	{ P_A53PLL, 5 },

And then this is not really doing much. So I wonder why we really
even need a parent_map? More like we need a map from parent_names
to mux number. We don't need to map some random enum into another
number space like we do for RCGs. I think we may have the same
problem with another qcom clk patch (see commit df964016490b in
clk-next). We really don't need the rcg version of parent_map in
either case, more like we just need a u8 *table (or u32
whatever), and then we're done. I'm going to make that change now
because otherwise we get into a mess with the parent_map stuff in
the other branch. I'll go clean up that too so we don't move
parent_map around.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v11 4/6] clk: qcom: Add regmap mux-div clocks support
  2017-12-05 15:46 ` [PATCH v11 4/6] clk: qcom: Add regmap mux-div clocks support Georgi Djakov
@ 2017-12-29  0:01   ` Stephen Boyd
  0 siblings, 0 replies; 27+ messages in thread
From: Stephen Boyd @ 2017-12-29  0:01 UTC (permalink / raw)
  To: Georgi Djakov
  Cc: jassisinghbrar, bjorn.andersson, mturquette, robh, linux-clk,
	linux-kernel, linux-arm-msm, devicetree

On 12/05, Georgi Djakov wrote:
> Add support for hardware that can switch both parent clock and divider
> at the same time. This avoids generating intermediate frequencies from
> either the old parent clock and new divider or new parent clock and
> old divider combinations.
> 
> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
> ---

Applied to clk-next

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* Re: [PATCH v11 3/6] clk: qcom: Add A53 PLL support
       [not found]   ` <20171205154701.27730-4-georgi.djakov-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
@ 2017-12-29  0:01     ` Stephen Boyd
  0 siblings, 0 replies; 27+ messages in thread
From: Stephen Boyd @ 2017-12-29  0:01 UTC (permalink / raw)
  To: Georgi Djakov
  Cc: jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w,
	bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A,
	mturquette-rdvid1DuHRBWk0Htik3J/w, robh-DgEjT+Ai2ygdnm+yROfE0A,
	linux-clk-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

On 12/05, Georgi Djakov wrote:
> The CPUs on Qualcomm MSM8916-based platforms are clocked by two PLLs,
> a primary (A53) CPU PLL and a secondary fixed-rate GPLL0. These sources
> are connected to a mux and half-integer divider, which is feeding the
> CPU cores.
> 
> This patch adds support for the primary CPU PLL which generates the
> higher range of frequencies above 1GHz.
> 
> Signed-off-by: Georgi Djakov <georgi.djakov-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Acked-by: Bjorn Andersson <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---

Applied to clk-next

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v11 2/6] mailbox: qcom: Create APCS child device for clock controller
  2017-12-24  5:06       ` Bjorn Andersson
@ 2017-12-29  6:14         ` Jassi Brar
  2018-01-04 16:56           ` Georgi Djakov
  0 siblings, 1 reply; 27+ messages in thread
From: Jassi Brar @ 2017-12-29  6:14 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Georgi Djakov, Stephen Boyd, Michael Turquette, Rob Herring,
	linux-clk, Linux Kernel Mailing List, linux-arm-msm,
	Devicetree List

Hi Bjorn,

On Sun, Dec 24, 2017 at 10:36 AM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> On Fri 22 Dec 20:57 PST 2017, Jassi Brar wrote:
>
>> On Tue, Dec 5, 2017 at 9:16 PM, Georgi Djakov <georgi.djakov@linaro.org> wrote:
>> > There is a clock controller functionality provided by the APCS hardware
>> > block of msm8916 devices. The device-tree would represent an APCS node
>> > with both mailbox and clock provider properties.
>> >
>> The spec might depict a 'clock' box and 'mailbox' box inside the
>> bigger APCS box. However, from the code I see in this patchset, they
>> are orthogonal and can & should be represented as independent DT
>> nodes.
>
> The APCS consists of a number of different hardware blocks, one of them
> being the "APCS global" block, which is what this node and drivers
> relate to. On 8916 this contains both the IPC register and clock
> control. But it's still just one block according to the hardware
> specification.
>
> As such DT should describe the one hardware block by one node IMHO.
>
In my even humbler opinion, DT should describe a h/w functional unit
which _could_ be seen as a standalone component.
For example, if this APCS had a mac controller, would we also populate
a netdev from mailbox driver? And what if next revision moves/drops
this clock controller out of APCS, keeping mailbox controller exactly
same?

Maybe some DT maintainer could enlighten either of us.

Cheers!

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

* Re: [PATCH v11 2/6] mailbox: qcom: Create APCS child device for clock controller
  2017-12-29  6:14         ` Jassi Brar
@ 2018-01-04 16:56           ` Georgi Djakov
  2018-01-27  3:44             ` Jassi Brar
  0 siblings, 1 reply; 27+ messages in thread
From: Georgi Djakov @ 2018-01-04 16:56 UTC (permalink / raw)
  To: Jassi Brar, Bjorn Andersson
  Cc: Stephen Boyd, Michael Turquette, Rob Herring, linux-clk,
	Linux Kernel Mailing List, linux-arm-msm, Devicetree List

Hi Jassi,

On 12/29/2017 08:14 AM, Jassi Brar wrote:
> Hi Bjorn,
> 
> On Sun, Dec 24, 2017 at 10:36 AM, Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
>> On Fri 22 Dec 20:57 PST 2017, Jassi Brar wrote:
>>
>>> On Tue, Dec 5, 2017 at 9:16 PM, Georgi Djakov <georgi.djakov@linaro.org> wrote:
>>>> There is a clock controller functionality provided by the APCS hardware
>>>> block of msm8916 devices. The device-tree would represent an APCS node
>>>> with both mailbox and clock provider properties.
>>>>
>>> The spec might depict a 'clock' box and 'mailbox' box inside the
>>> bigger APCS box. However, from the code I see in this patchset, they
>>> are orthogonal and can & should be represented as independent DT
>>> nodes.
>>
>> The APCS consists of a number of different hardware blocks, one of them
>> being the "APCS global" block, which is what this node and drivers
>> relate to. On 8916 this contains both the IPC register and clock
>> control. But it's still just one block according to the hardware
>> specification.
>>
>> As such DT should describe the one hardware block by one node IMHO.
>>
> In my even humbler opinion, DT should describe a h/w functional unit
> which _could_ be seen as a standalone component.

The APCS is one separate register block related to the CPU cluster. I
haven't seen any strict guidelines for such cases in the DT docs, and
during the discussion got the impression that this is the preferred
binding. Rob has also reviewed the binding, so we should be fine to move
forward with this one.

> For example, if this APCS had a mac controller, would we also populate
> a netdev from mailbox driver? And what if next revision moves/drops
> this clock controller out of APCS, keeping mailbox controller exactly
> same?

The clock controller may change in some next SoC architecture and that's
why the SoC version is also part of the the compatible string.

Thanks,
Georgi

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

* Re: [PATCH v11 0/6] Add support for Qualcomm A53 CPU clock
  2017-12-05 15:46 [PATCH v11 0/6] Add support for Qualcomm A53 CPU clock Georgi Djakov
                   ` (7 preceding siblings ...)
  2017-12-22  0:49 ` Stephen Boyd
@ 2018-01-26 14:30 ` Georgi Djakov
  8 siblings, 0 replies; 27+ messages in thread
From: Georgi Djakov @ 2018-01-26 14:30 UTC (permalink / raw)
  To: jassisinghbrar
  Cc: sboyd, bjorn.andersson, mturquette, robh, linux-clk,
	linux-kernel, linux-arm-msm, devicetree

On 12/05/2017 05:46 PM, Georgi Djakov wrote:
> This patchset adds support for the A53 CPU clock on MSM8916 platforms
> and allows scaling of the CPU frequency on msm8916 based platforms.
> 
> Changes since v10 (https://lkml.org/lkml/2017/12/1/577)
> * Addressed Bjorn's comments on APCS clock driver.
> * Picked Acks from Rob and Bjorn.
> 
[..]
> 
> Georgi Djakov (6):
>   mailbox: qcom: Convert APCS IPC driver to use regmap
>   mailbox: qcom: Create APCS child device for clock controller
>   clk: qcom: Add A53 PLL support
>   clk: qcom: Add regmap mux-div clocks support
>   dt-bindings: mailbox: qcom: Document the APCS clock binding
>   clk: qcom: Add APCS clock controller support
> 

Hi Jassi,

Do you want me to re-send the pending mailbox patches?

Thanks,
Georgi

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

* Re: [PATCH v11 2/6] mailbox: qcom: Create APCS child device for clock controller
  2018-01-04 16:56           ` Georgi Djakov
@ 2018-01-27  3:44             ` Jassi Brar
  2018-01-31 18:40               ` Georgi Djakov
  0 siblings, 1 reply; 27+ messages in thread
From: Jassi Brar @ 2018-01-27  3:44 UTC (permalink / raw)
  To: Georgi Djakov
  Cc: Bjorn Andersson, Stephen Boyd, Michael Turquette, Rob Herring,
	linux-clk, Linux Kernel Mailing List, linux-arm-msm,
	Devicetree List

On Thu, Jan 4, 2018 at 10:26 PM, Georgi Djakov <georgi.djakov@linaro.org> wrote:
> Hi Jassi,
>
> On 12/29/2017 08:14 AM, Jassi Brar wrote:
>> Hi Bjorn,
>>
>> On Sun, Dec 24, 2017 at 10:36 AM, Bjorn Andersson
>> <bjorn.andersson@linaro.org> wrote:
>>> On Fri 22 Dec 20:57 PST 2017, Jassi Brar wrote:
>>>
>>>> On Tue, Dec 5, 2017 at 9:16 PM, Georgi Djakov <georgi.djakov@linaro.org> wrote:
>>>>> There is a clock controller functionality provided by the APCS hardware
>>>>> block of msm8916 devices. The device-tree would represent an APCS node
>>>>> with both mailbox and clock provider properties.
>>>>>
>>>> The spec might depict a 'clock' box and 'mailbox' box inside the
>>>> bigger APCS box. However, from the code I see in this patchset, they
>>>> are orthogonal and can & should be represented as independent DT
>>>> nodes.
>>>
>>> The APCS consists of a number of different hardware blocks, one of them
>>> being the "APCS global" block, which is what this node and drivers
>>> relate to. On 8916 this contains both the IPC register and clock
>>> control. But it's still just one block according to the hardware
>>> specification.
>>>
>>> As such DT should describe the one hardware block by one node IMHO.
>>>
>> In my even humbler opinion, DT should describe a h/w functional unit
>> which _could_ be seen as a standalone component.
>
> The APCS is one separate register block related to the CPU cluster. I
> haven't seen any strict guidelines for such cases in the DT docs, and
> during the discussion got the impression that this is the preferred
> binding. Rob has also reviewed the binding, so we should be fine to move
> forward with this one.
>
Well, I can't overrule Rob. But I am really not happy with random
device spawning from mailbox drivers. I know there are such instances
already in the kernel but that doesn't make it legit... unless there
is some hard dependency. Is there?

>> For example, if this APCS had a mac controller, would we also populate
>> a netdev from mailbox driver? And what if next revision moves/drops
>> this clock controller out of APCS, keeping mailbox controller exactly
>> same?
>
> The clock controller may change in some next SoC architecture and that's
> why the SoC version is also part of the the compatible string.
>
So the mailbox driver will be updated to spawn yet another type of clock?
And again for next revision and so on... I know that is unlikely but
the point is why not have separate clock drivers for independent h/w
clocks?
I'll let Rob take the final call.

Cheers!

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

* Re: [PATCH v11 2/6] mailbox: qcom: Create APCS child device for clock controller
  2018-01-27  3:44             ` Jassi Brar
@ 2018-01-31 18:40               ` Georgi Djakov
  2018-02-01  6:57                 ` Jassi Brar
  0 siblings, 1 reply; 27+ messages in thread
From: Georgi Djakov @ 2018-01-31 18:40 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Bjorn Andersson, Stephen Boyd, Michael Turquette, Rob Herring,
	linux-clk, Linux Kernel Mailing List, linux-arm-msm,
	Devicetree List

Hi Jassi,

On 01/27/2018 05:44 AM, Jassi Brar wrote:
> On Thu, Jan 4, 2018 at 10:26 PM, Georgi Djakov <georgi.djakov@linaro.org> wrote:
>> Hi Jassi,
>>
>> On 12/29/2017 08:14 AM, Jassi Brar wrote:
>>> Hi Bjorn,
>>>
>>> On Sun, Dec 24, 2017 at 10:36 AM, Bjorn Andersson
>>> <bjorn.andersson@linaro.org> wrote:
>>>> On Fri 22 Dec 20:57 PST 2017, Jassi Brar wrote:
>>>>
>>>>> On Tue, Dec 5, 2017 at 9:16 PM, Georgi Djakov <georgi.djakov@linaro.org> wrote:
>>>>>> There is a clock controller functionality provided by the APCS hardware
>>>>>> block of msm8916 devices. The device-tree would represent an APCS node
>>>>>> with both mailbox and clock provider properties.
>>>>>>
>>>>> The spec might depict a 'clock' box and 'mailbox' box inside the
>>>>> bigger APCS box. However, from the code I see in this patchset, they
>>>>> are orthogonal and can & should be represented as independent DT
>>>>> nodes.
>>>>
>>>> The APCS consists of a number of different hardware blocks, one of them
>>>> being the "APCS global" block, which is what this node and drivers
>>>> relate to. On 8916 this contains both the IPC register and clock
>>>> control. But it's still just one block according to the hardware
>>>> specification.
>>>>
>>>> As such DT should describe the one hardware block by one node IMHO.
>>>>
>>> In my even humbler opinion, DT should describe a h/w functional unit
>>> which _could_ be seen as a standalone component.
>>
>> The APCS is one separate register block related to the CPU cluster. I
>> haven't seen any strict guidelines for such cases in the DT docs, and
>> during the discussion got the impression that this is the preferred
>> binding. Rob has also reviewed the binding, so we should be fine to move
>> forward with this one.
>>
> Well, I can't overrule Rob. But I am really not happy with random
> device spawning from mailbox drivers. I know there are such instances
> already in the kernel but that doesn't make it legit... unless there
> is some hard dependency. Is there?

The dependency is that on this SoC, these functionalities are combined
into this "CPU subsystem" block.

Initially APCS was a syscon and it was discussed it an year ago in this
mail thread [1] and we are trying to move away from the syscon and come
up with some bindings. During v8 and v9 of this patchset i have switched
from multiple child nodes to a single node as this seem to be the
preferred form.

>>> For example, if this APCS had a mac controller, would we also populate
>>> a netdev from mailbox driver? And what if next revision moves/drops
>>> this clock controller out of APCS, keeping mailbox controller exactly
>>> same?
>>
>> The clock controller may change in some next SoC architecture and that's
>> why the SoC version is also part of the the compatible string.
>>
> So the mailbox driver will be updated to spawn yet another type of clock?
> And again for next revision and so on... I know that is unlikely but
> the point is why not have separate clock drivers for independent h/w
> clocks?

Each revision re-shuffles the APCS block, so at least the offsets would
be different. I don't see anything bad with spawning a child device, but
what are the other options? Should we go back on the bindings and
suggest something new all over again? Or do you have any other idea?
Could you please explain your point further?

Thanks,
Georgi

[1] https://lkml.org/lkml/2017/3/20/991

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

* Re: [PATCH v11 2/6] mailbox: qcom: Create APCS child device for clock controller
  2018-01-31 18:40               ` Georgi Djakov
@ 2018-02-01  6:57                 ` Jassi Brar
  2018-02-01  8:01                   ` Georgi Djakov
  0 siblings, 1 reply; 27+ messages in thread
From: Jassi Brar @ 2018-02-01  6:57 UTC (permalink / raw)
  To: Georgi Djakov
  Cc: Bjorn Andersson, Stephen Boyd, Michael Turquette, Rob Herring,
	linux-clk, Linux Kernel Mailing List, linux-arm-msm,
	Devicetree List

On Thu, Feb 1, 2018 at 12:10 AM, Georgi Djakov <georgi.djakov@linaro.org> wrote:
>
> Hi Jassi,
>
> On 01/27/2018 05:44 AM, Jassi Brar wrote:
> > On Thu, Jan 4, 2018 at 10:26 PM, Georgi Djakov <georgi.djakov@linaro.org> wrote:
> >> Hi Jassi,
> >>
> >> On 12/29/2017 08:14 AM, Jassi Brar wrote:
> >>> Hi Bjorn,
> >>>
> >>> On Sun, Dec 24, 2017 at 10:36 AM, Bjorn Andersson
> >>> <bjorn.andersson@linaro.org> wrote:
> >>>> On Fri 22 Dec 20:57 PST 2017, Jassi Brar wrote:
> >>>>
> >>>>> On Tue, Dec 5, 2017 at 9:16 PM, Georgi Djakov <georgi.djakov@linaro.org> wrote:
> >>>>>> There is a clock controller functionality provided by the APCS hardware
> >>>>>> block of msm8916 devices. The device-tree would represent an APCS node
> >>>>>> with both mailbox and clock provider properties.
> >>>>>>
> >>>>> The spec might depict a 'clock' box and 'mailbox' box inside the
> >>>>> bigger APCS box. However, from the code I see in this patchset, they
> >>>>> are orthogonal and can & should be represented as independent DT
> >>>>> nodes.
> >>>>
> >>>> The APCS consists of a number of different hardware blocks, one of them
> >>>> being the "APCS global" block, which is what this node and drivers
> >>>> relate to. On 8916 this contains both the IPC register and clock
> >>>> control. But it's still just one block according to the hardware
> >>>> specification.
> >>>>
> >>>> As such DT should describe the one hardware block by one node IMHO.
> >>>>
> >>> In my even humbler opinion, DT should describe a h/w functional unit
> >>> which _could_ be seen as a standalone component.
> >>
> >> The APCS is one separate register block related to the CPU cluster. I
> >> haven't seen any strict guidelines for such cases in the DT docs, and
> >> during the discussion got the impression that this is the preferred
> >> binding. Rob has also reviewed the binding, so we should be fine to move
> >> forward with this one.
> >>
> > Well, I can't overrule Rob. But I am really not happy with random
> > device spawning from mailbox drivers. I know there are such instances
> > already in the kernel but that doesn't make it legit... unless there
> > is some hard dependency. Is there?
>
> The dependency is that on this SoC, these functionalities are combined
> into this "CPU subsystem" block.
>
I see the register space is shared between mailbox and the clock. So I
guess, yes, simply creating a device here and passing the common
regmap is tidier. Which patches are already picked up?

Cheers!

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

* Re: [PATCH v11 2/6] mailbox: qcom: Create APCS child device for clock controller
  2018-02-01  6:57                 ` Jassi Brar
@ 2018-02-01  8:01                   ` Georgi Djakov
  0 siblings, 0 replies; 27+ messages in thread
From: Georgi Djakov @ 2018-02-01  8:01 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Bjorn Andersson, Stephen Boyd, Michael Turquette, Rob Herring,
	linux-clk, Linux Kernel Mailing List, linux-arm-msm,
	Devicetree List

On 02/01/2018 08:57 AM, Jassi Brar wrote:
> On Thu, Feb 1, 2018 at 12:10 AM, Georgi Djakov <georgi.djakov@linaro.org> wrote:
>>
>> Hi Jassi,
>>
>> On 01/27/2018 05:44 AM, Jassi Brar wrote:
>>> On Thu, Jan 4, 2018 at 10:26 PM, Georgi Djakov <georgi.djakov@linaro.org> wrote:
>>>> Hi Jassi,
>>>>
>>>> On 12/29/2017 08:14 AM, Jassi Brar wrote:
>>>>> Hi Bjorn,
>>>>>
>>>>> On Sun, Dec 24, 2017 at 10:36 AM, Bjorn Andersson
>>>>> <bjorn.andersson@linaro.org> wrote:
>>>>>> On Fri 22 Dec 20:57 PST 2017, Jassi Brar wrote:
>>>>>>
>>>>>>> On Tue, Dec 5, 2017 at 9:16 PM, Georgi Djakov <georgi.djakov@linaro.org> wrote:
>>>>>>>> There is a clock controller functionality provided by the APCS hardware
>>>>>>>> block of msm8916 devices. The device-tree would represent an APCS node
>>>>>>>> with both mailbox and clock provider properties.
>>>>>>>>
>>>>>>> The spec might depict a 'clock' box and 'mailbox' box inside the
>>>>>>> bigger APCS box. However, from the code I see in this patchset, they
>>>>>>> are orthogonal and can & should be represented as independent DT
>>>>>>> nodes.
>>>>>>
>>>>>> The APCS consists of a number of different hardware blocks, one of them
>>>>>> being the "APCS global" block, which is what this node and drivers
>>>>>> relate to. On 8916 this contains both the IPC register and clock
>>>>>> control. But it's still just one block according to the hardware
>>>>>> specification.
>>>>>>
>>>>>> As such DT should describe the one hardware block by one node IMHO.
>>>>>>
>>>>> In my even humbler opinion, DT should describe a h/w functional unit
>>>>> which _could_ be seen as a standalone component.
>>>>
>>>> The APCS is one separate register block related to the CPU cluster. I
>>>> haven't seen any strict guidelines for such cases in the DT docs, and
>>>> during the discussion got the impression that this is the preferred
>>>> binding. Rob has also reviewed the binding, so we should be fine to move
>>>> forward with this one.
>>>>
>>> Well, I can't overrule Rob. But I am really not happy with random
>>> device spawning from mailbox drivers. I know there are such instances
>>> already in the kernel but that doesn't make it legit... unless there
>>> is some hard dependency. Is there?
>>
>> The dependency is that on this SoC, these functionalities are combined
>> into this "CPU subsystem" block.
>>
> I see the register space is shared between mailbox and the clock. So I
> guess, yes, simply creating a device here and passing the common
> regmap is tidier. Which patches are already picked up?

Patches 3, 4 and 6 are already picked into the clk tree. Still pending
are patches 1, 2 and 5.

Thanks,
Georgi

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

end of thread, other threads:[~2018-02-01  8:01 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-05 15:46 [PATCH v11 0/6] Add support for Qualcomm A53 CPU clock Georgi Djakov
2017-12-05 15:46 ` [PATCH v11 1/6] mailbox: qcom: Convert APCS IPC driver to use regmap Georgi Djakov
2017-12-05 15:46 ` [PATCH v11 2/6] mailbox: qcom: Create APCS child device for clock controller Georgi Djakov
2017-12-23  4:57   ` Jassi Brar
     [not found]     ` <CABb+yY146ho0gtbUzDv-Z_xESkU2y=UYfBA6i+522+m58LkABQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-12-24  5:06       ` Bjorn Andersson
2017-12-29  6:14         ` Jassi Brar
2018-01-04 16:56           ` Georgi Djakov
2018-01-27  3:44             ` Jassi Brar
2018-01-31 18:40               ` Georgi Djakov
2018-02-01  6:57                 ` Jassi Brar
2018-02-01  8:01                   ` Georgi Djakov
2017-12-05 15:46 ` [PATCH v11 3/6] clk: qcom: Add A53 PLL support Georgi Djakov
     [not found]   ` <20171205154701.27730-4-georgi.djakov-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2017-12-29  0:01     ` Stephen Boyd
2017-12-05 15:46 ` [PATCH v11 4/6] clk: qcom: Add regmap mux-div clocks support Georgi Djakov
2017-12-29  0:01   ` Stephen Boyd
2017-12-05 15:47 ` [PATCH v11 5/6] dt-bindings: mailbox: qcom: Document the APCS clock binding Georgi Djakov
     [not found] ` <20171205154701.27730-1-georgi.djakov-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2017-12-05 15:47   ` [PATCH v11 6/6] clk: qcom: Add APCS clock controller support Georgi Djakov
     [not found]     ` <20171205154701.27730-7-georgi.djakov-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2017-12-05 17:21       ` Bjorn Andersson
2017-12-29  0:00       ` Stephen Boyd
2017-12-06  5:51   ` [PATCH v11 0/6] Add support for Qualcomm A53 CPU clock Amit Kucheria
2017-12-07 16:00     ` Georgi Djakov
2017-12-06 21:08 ` Rob Herring
2017-12-07 16:02   ` Georgi Djakov
2017-12-22  0:49 ` Stephen Boyd
2017-12-22 14:52   ` Georgi Djakov
2017-12-28 23:53     ` Stephen Boyd
2018-01-26 14:30 ` Georgi Djakov

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