This patchset adds support for the A53 CPU clock and allows scaling
of the CPU frequency on msm8916 based platforms.
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 (3):
clk: qcom: Add A53 PLL support
clk: qcom: Add regmap mux-div clocks support
mailbox: qcom: Add support for APCS clock controller
.../devicetree/bindings/clock/qcom,a53pll.txt | 22 ++
.../bindings/mailbox/qcom,apcs-kpss-global.txt | 5 +
drivers/clk/qcom/Kconfig | 9 +
drivers/clk/qcom/Makefile | 2 +
drivers/clk/qcom/a53-pll.c | 94 ++++++++
drivers/clk/qcom/clk-regmap-mux-div.c | 237 +++++++++++++++++++++
drivers/clk/qcom/clk-regmap-mux-div.h | 52 +++++
drivers/mailbox/qcom-apcs-ipc-mailbox.c | 122 +++++++++++
8 files changed, 543 insertions(+)
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/clk-regmap-mux-div.c
create mode 100644 drivers/clk/qcom/clk-regmap-mux-div.h
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 <[email protected]>
---
drivers/clk/qcom/Makefile | 1 +
drivers/clk/qcom/clk-regmap-mux-div.c | 237 ++++++++++++++++++++++++++++++++++
drivers/clk/qcom/clk-regmap-mux-div.h | 52 ++++++++
3 files changed, 290 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 19ae884b5166..ac38c2b21847 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -9,6 +9,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..5ec31ec3efa7
--- /dev/null
+++ b/drivers/clk/qcom/clk-regmap-mux-div.c
@@ -0,0 +1,237 @@
+/*
+ * Copyright (c) 2017, Linaro Limited
+ * Copyright (c) 2014, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#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..3380e8f6e8a1
--- /dev/null
+++ b/drivers/clk/qcom/clk-regmap-mux-div.h
@@ -0,0 +1,52 @@
+/*
+ * Copyright (c) 2015, Linaro Limited
+ * Copyright (c) 2014, The Linux Foundation. All rights reserved.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#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
+ * @clk_nb: clock notifier registered for clock rate changes of the A53 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 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
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 <[email protected]>
---
.../bindings/mailbox/qcom,apcs-kpss-global.txt | 5 +
drivers/mailbox/qcom-apcs-ipc-mailbox.c | 122 +++++++++++++++++++++
2 files changed, 127 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..2432be307083 100644
--- a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt
+++ b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt
@@ -21,6 +21,11 @@ platforms.
Value type: <u32>
Definition: as described in mailbox.txt, must be 1
+- #clock-cells:
+ Usage: required for msm8916 platforms
+ Value type: <u32>
+ Definition: as described in clock-bindings.txt, must be 0
+
= EXAMPLE
The following example describes the APCS HMSS found in MSM8996 and part of the
diff --git a/drivers/mailbox/qcom-apcs-ipc-mailbox.c b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
index 9924c6d7f05d..da363c6580da 100644
--- a/drivers/mailbox/qcom-apcs-ipc-mailbox.c
+++ b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
@@ -11,6 +11,8 @@
* GNU General Public License for more details.
*/
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/io.h>
@@ -19,6 +21,34 @@
#include <linux/of_platform.h>
#include <linux/platform_device.h>
#include <linux/mailbox_controller.h>
+#include <linux/regmap.h>
+
+#include "../clk/qcom/clk-regmap.h"
+#include "../clk/qcom/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",
+};
+
+static const struct regmap_config a53cc_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .max_register = 0x1000,
+ .fast_io = true,
+ .val_format_endian = REGMAP_ENDIAN_LITTLE,
+};
#define QCOM_APCS_IPC_BITS 32
@@ -45,8 +75,93 @@ static const struct mbox_chan_ops qcom_apcs_ipc_ops = {
.send_data = qcom_apcs_ipc_send_data,
};
+/*
+ * 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 msm8916_register_clk(struct device *dev, void __iomem *base)
+{
+ struct clk_regmap_mux_div *a53cc;
+ struct clk *pclk;
+ struct regmap *regmap;
+ struct clk_init_data init = { };
+ int ret;
+
+ a53cc = devm_kzalloc(dev, sizeof(*a53cc), GFP_KERNEL);
+ if (!a53cc)
+ return -ENOMEM;
+
+ 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;
+
+ init.name = "a53mux";
+ init.parent_names = gpll0_a53cc;
+ init.num_parents = 2;
+ init.ops = &clk_regmap_mux_div_ops;
+ init.flags = CLK_SET_RATE_PARENT;
+ a53cc->clkr.hw.init = &init;
+
+ pclk = __clk_lookup(gpll0_a53cc[1]);
+ if (!pclk)
+ return -EPROBE_DEFER;
+
+ a53cc->clk_nb.notifier_call = a53cc_notifier_cb;
+ ret = clk_notifier_register(pclk, &a53cc->clk_nb);
+ if (ret) {
+ dev_err(dev, "failed to register clock notifier: %d\n", ret);
+ return ret;
+ }
+
+ regmap = devm_regmap_init_mmio(dev, base, &a53cc_regmap_config);
+ if (IS_ERR(regmap)) {
+ ret = PTR_ERR(regmap);
+ dev_err(dev, "failed to init regmap mmio: %d\n", ret);
+ goto err;
+ }
+
+ a53cc->clkr.regmap = regmap;
+
+ 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(dev->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;
+ }
+
+ return 0;
+
+err:
+ clk_notifier_unregister(pclk, &a53cc->clk_nb);
+ return ret;
+}
+
static int qcom_apcs_ipc_probe(struct platform_device *pdev)
{
+ struct device_node *np = pdev->dev.of_node;
struct qcom_apcs_ipc *apcs;
struct resource *res;
unsigned long offset;
@@ -63,6 +178,13 @@ static int qcom_apcs_ipc_probe(struct platform_device *pdev)
if (IS_ERR(base))
return PTR_ERR(base);
+ if (of_device_is_compatible(np, "qcom,msm8916-apcs-kpss-global")) {
+ /* register the APCS mux and divider clock */
+ ret = msm8916_register_clk(&pdev->dev, base);
+ if (ret)
+ return ret;
+ }
+
offset = (unsigned long)of_device_get_match_data(&pdev->dev);
apcs->reg = base + offset;
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 <[email protected]>
---
.../devicetree/bindings/clock/qcom,a53pll.txt | 22 +++++
drivers/clk/qcom/Kconfig | 9 +++
drivers/clk/qcom/Makefile | 1 +
drivers/clk/qcom/a53-pll.c | 94 ++++++++++++++++++++++
4 files changed, 126 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..f4c2fddf6e7f
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/qcom,a53pll.txt
@@ -0,0 +1,22 @@
+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..057cf60ed037 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -12,6 +12,15 @@ config COMMON_CLK_QCOM
select REGMAP_MMIO
select RESET_CONTROLLER
+config QCOM_A53PLL
+ bool "A53 PLL"
+ depends on COMMON_CLK_QCOM
+ help
+ Support for the A53 PLL on Qualcomm MSM8916 devices. It provides
+ support for CPU frequencies above 1GHz.
+ 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 3f3aff229fb7..19ae884b5166 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -31,5 +31,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..e039937e89fc
--- /dev/null
+++ b/drivers/clk/qcom/a53-pll.c
@@ -0,0 +1,94 @@
+/*
+ * Copyright (c) 2017, Linaro Limited
+ * Copyright (c) 2014, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#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, 65, 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,
+ .val_format_endian = REGMAP_ENDIAN_LITTLE,
+};
+
+static const struct of_device_id qcom_a53pll_match_table[] = {
+ { .compatible = "qcom,msm8916-a53pll" },
+ { }
+};
+
+static int qcom_a53pll_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct clk_pll *pll;
+ struct resource *res;
+ void __iomem *base;
+ struct regmap *regmap;
+ struct clk_init_data init = { };
+
+ 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;
+
+ return devm_clk_register_regmap(dev, &pll->clkr);
+}
+
+static struct platform_driver qcom_a53pll_driver = {
+ .probe = qcom_a53pll_probe,
+ .driver = {
+ .name = "qcom-a53pll",
+ .of_match_table = qcom_a53pll_match_table,
+ },
+};
+
+builtin_platform_driver(qcom_a53pll_driver);
On Fri 23 Jun 09:15 PDT 2017, Georgi Djakov wrote:
> +static int msm8916_register_clk(struct device *dev, void __iomem *base)
> +{
[..]
> + regmap = devm_regmap_init_mmio(dev, base, &a53cc_regmap_config);
> + if (IS_ERR(regmap)) {
> + ret = PTR_ERR(regmap);
> + dev_err(dev, "failed to init regmap mmio: %d\n", ret);
> + goto err;
> + }
I think it would be cleaner if you create the regmap in probe() and we
use that throughout the driver - rather than using two different access
mechanism.
> +
> + a53cc->clkr.regmap = regmap;
> +
> + 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(dev->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;
> + }
> +
> + return 0;
> +
> +err:
> + clk_notifier_unregister(pclk, &a53cc->clk_nb);
> + return ret;
> +}
> +
> static int qcom_apcs_ipc_probe(struct platform_device *pdev)
> {
> + struct device_node *np = pdev->dev.of_node;
> struct qcom_apcs_ipc *apcs;
> struct resource *res;
> unsigned long offset;
> @@ -63,6 +178,13 @@ static int qcom_apcs_ipc_probe(struct platform_device *pdev)
> if (IS_ERR(base))
> return PTR_ERR(base);
>
> + if (of_device_is_compatible(np, "qcom,msm8916-apcs-kpss-global")) {
> + /* register the APCS mux and divider clock */
> + ret = msm8916_register_clk(&pdev->dev, base);
> + if (ret)
> + return ret;
> + }
> +
Don't you need to clean up anything in the below error path and in
remove()?
> offset = (unsigned long)of_device_get_match_data(&pdev->dev);
>
> apcs->reg = base + offset;
Other than that I think this looks reasonable.
Regards,
Bjorn
Hi Georgi,
[auto build test ERROR on next-20170619]
[cannot apply to clk/clk-next robh/for-next linus/master v4.12-rc6 v4.12-rc5 v4.12-rc4 v4.12-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Georgi-Djakov/Add-support-for-Qualcomm-A53-CPU-clock/20170625-063544
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 6.2.0
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=ia64
All error/warnings (new ones prefixed by >>):
In file included from drivers/mailbox/qcom-apcs-ipc-mailbox.c:26:0:
>> drivers/mailbox/../clk/qcom/clk-regmap.h:31:16: error: field 'hw' has incomplete type
struct clk_hw hw;
^~
drivers/mailbox/qcom-apcs-ipc-mailbox.c: In function 'msm8916_register_clk':
>> drivers/mailbox/qcom-apcs-ipc-mailbox.c:101:9: error: variable 'init' has initializer but incomplete type
struct clk_init_data init = { };
^~~~~~~~~~~~~
>> drivers/mailbox/qcom-apcs-ipc-mailbox.c:101:23: error: storage size of 'init' isn't known
struct clk_init_data init = { };
^~~~
>> drivers/mailbox/qcom-apcs-ipc-mailbox.c:119:15: error: 'CLK_SET_RATE_PARENT' undeclared (first use in this function)
init.flags = CLK_SET_RATE_PARENT;
^~~~~~~~~~~~~~~~~~~
drivers/mailbox/qcom-apcs-ipc-mailbox.c:119:15: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/mailbox/qcom-apcs-ipc-mailbox.c:122:9: error: implicit declaration of function '__clk_lookup' [-Werror=implicit-function-declaration]
pclk = __clk_lookup(gpll0_a53cc[1]);
^~~~~~~~~~~~
>> drivers/mailbox/qcom-apcs-ipc-mailbox.c:122:7: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
pclk = __clk_lookup(gpll0_a53cc[1]);
^
>> drivers/mailbox/qcom-apcs-ipc-mailbox.c:148:8: error: implicit declaration of function 'of_clk_add_hw_provider' [-Werror=implicit-function-declaration]
ret = of_clk_add_hw_provider(dev->of_node, of_clk_hw_simple_get,
^~~~~~~~~~~~~~~~~~~~~~
>> drivers/mailbox/qcom-apcs-ipc-mailbox.c:148:45: error: 'of_clk_hw_simple_get' undeclared (first use in this function)
ret = of_clk_add_hw_provider(dev->of_node, of_clk_hw_simple_get,
^~~~~~~~~~~~~~~~~~~~
drivers/mailbox/qcom-apcs-ipc-mailbox.c:101:23: warning: unused variable 'init' [-Wunused-variable]
struct clk_init_data init = { };
^~~~
cc1: some warnings being treated as errors
vim +/hw +31 drivers/mailbox/../clk/qcom/clk-regmap.h
085d7a45 Stephen Boyd 2014-01-15 25 * @enable_reg: register when using regmap enable/disable ops
085d7a45 Stephen Boyd 2014-01-15 26 * @enable_mask: mask when using regmap enable/disable ops
085d7a45 Stephen Boyd 2014-01-15 27 * @enable_is_inverted: flag to indicate set enable_mask bits to disable
085d7a45 Stephen Boyd 2014-01-15 28 * when using clock_enable_regmap and friends APIs.
085d7a45 Stephen Boyd 2014-01-15 29 */
085d7a45 Stephen Boyd 2014-01-15 30 struct clk_regmap {
085d7a45 Stephen Boyd 2014-01-15 @31 struct clk_hw hw;
085d7a45 Stephen Boyd 2014-01-15 32 struct regmap *regmap;
085d7a45 Stephen Boyd 2014-01-15 33 unsigned int enable_reg;
085d7a45 Stephen Boyd 2014-01-15 34 unsigned int enable_mask;
:::::: The code at line 31 was first introduced by commit
:::::: 085d7a455444f4d425371ee3c8a273c6e1b522db clk: qcom: Add a regmap type clock struct
:::::: TO: Stephen Boyd <[email protected]>
:::::: CC: Mike Turquette <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Georgi,
[auto build test ERROR on next-20170619]
[cannot apply to clk/clk-next robh/for-next linus/master v4.12-rc6 v4.12-rc5 v4.12-rc4 v4.12-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Georgi-Djakov/Add-support-for-Qualcomm-A53-CPU-clock/20170625-063544
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
>> ERROR: "__mux_div_set_src_div" [drivers/mailbox/qcom-apcs-ipc-mailbox.ko] undefined!
>> ERROR: "__clk_lookup" [drivers/mailbox/qcom-apcs-ipc-mailbox.ko] undefined!
>> ERROR: "clk_regmap_mux_div_ops" [drivers/mailbox/qcom-apcs-ipc-mailbox.ko] undefined!
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Fri, Jun 23, 2017 at 07:15:31PM +0300, 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 <[email protected]>
> ---
> .../devicetree/bindings/clock/qcom,a53pll.txt | 22 +++++
It's preferred to split bindings to a separate patch. In any case,
Acked-by: Rob Herring <[email protected]>
One kconfig comment though:
> drivers/clk/qcom/Kconfig | 9 +++
> drivers/clk/qcom/Makefile | 1 +
> drivers/clk/qcom/a53-pll.c | 94 ++++++++++++++++++++++
> 4 files changed, 126 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..f4c2fddf6e7f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/qcom,a53pll.txt
> @@ -0,0 +1,22 @@
> +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..057cf60ed037 100644
> --- a/drivers/clk/qcom/Kconfig
> +++ b/drivers/clk/qcom/Kconfig
> @@ -12,6 +12,15 @@ config COMMON_CLK_QCOM
> select REGMAP_MMIO
> select RESET_CONTROLLER
>
> +config QCOM_A53PLL
> + bool "A53 PLL"
Figuring out config options needed for a specific QC SoC is "fun". If
this is only for MSM8916, then add that to the config option or prompt
text at least.
And please update the arm64 defconfig with this option.
> + depends on COMMON_CLK_QCOM
> + help
> + Support for the A53 PLL on Qualcomm MSM8916 devices. It provides
> + support for CPU frequencies above 1GHz.
> + Say Y if you want to support CPU frequency scaling on devices
> + such as MSM8916.
On Fri, Jun 23, 2017 at 07:15:33PM +0300, 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.
>
> Signed-off-by: Georgi Djakov <[email protected]>
> ---
> .../bindings/mailbox/qcom,apcs-kpss-global.txt | 5 +
Acked-by: Rob Herring <[email protected]>
> drivers/mailbox/qcom-apcs-ipc-mailbox.c | 122 +++++++++++++++++++++
> 2 files changed, 127 insertions(+)
On 06/23, Georgi Djakov wrote:
> diff --git a/drivers/mailbox/qcom-apcs-ipc-mailbox.c b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
> index 9924c6d7f05d..da363c6580da 100644
> --- a/drivers/mailbox/qcom-apcs-ipc-mailbox.c
> +++ b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
> @@ -11,6 +11,8 @@
> * GNU General Public License for more details.
> */
>
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/io.h>
> @@ -19,6 +21,34 @@
> #include <linux/of_platform.h>
> #include <linux/platform_device.h>
> #include <linux/mailbox_controller.h>
> +#include <linux/regmap.h>
> +
> +#include "../clk/qcom/clk-regmap.h"
> +#include "../clk/qcom/clk-regmap-mux-div.h"
Why?
> +
> +
> +static int msm8916_register_clk(struct device *dev, void __iomem *base)
> +{
> + struct clk_regmap_mux_div *a53cc;
> + struct clk *pclk;
> + struct regmap *regmap;
> + struct clk_init_data init = { };
> + int ret;
> +
> + a53cc = devm_kzalloc(dev, sizeof(*a53cc), GFP_KERNEL);
> + if (!a53cc)
> + return -ENOMEM;
> +
> + 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;
> +
> + init.name = "a53mux";
> + init.parent_names = gpll0_a53cc;
> + init.num_parents = 2;
ARRAY_SIZE(gpll0_a53cc) instead of 2 please
> + init.ops = &clk_regmap_mux_div_ops;
> + init.flags = CLK_SET_RATE_PARENT;
> + a53cc->clkr.hw.init = &init;
> +
> + pclk = __clk_lookup(gpll0_a53cc[1]);
Urgh.. ok. We can't clk_get()?
> + if (!pclk)
> + return -EPROBE_DEFER;
> +
> + a53cc->clk_nb.notifier_call = a53cc_notifier_cb;
> + ret = clk_notifier_register(pclk, &a53cc->clk_nb);
> + if (ret) {
> + dev_err(dev, "failed to register clock notifier: %d\n", ret);
> + return ret;
> + }
> +
> + regmap = devm_regmap_init_mmio(dev, base, &a53cc_regmap_config);
> + if (IS_ERR(regmap)) {
> + ret = PTR_ERR(regmap);
> + dev_err(dev, "failed to init regmap mmio: %d\n", ret);
> + goto err;
> + }
> +
> + a53cc->clkr.regmap = regmap;
> +
> + ret = devm_clk_register_regmap(dev, &a53cc->clkr);
Regmap is not a requirement to work with the qcom clk driver.
> + if (ret) {
> + dev_err(dev, "failed to register regmap clock: %d\n", ret);
> + goto err;
> + }
> +
> + ret = of_clk_add_hw_provider(dev->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;
> + }
> +
> + return 0;
> +
> +err:
> + clk_notifier_unregister(pclk, &a53cc->clk_nb);
> + return ret;
> +}
> +
> static int qcom_apcs_ipc_probe(struct platform_device *pdev)
> {
> + struct device_node *np = pdev->dev.of_node;
> struct qcom_apcs_ipc *apcs;
> struct resource *res;
> unsigned long offset;
> @@ -63,6 +178,13 @@ static int qcom_apcs_ipc_probe(struct platform_device *pdev)
> if (IS_ERR(base))
> return PTR_ERR(base);
>
> + if (of_device_is_compatible(np, "qcom,msm8916-apcs-kpss-global")) {
> + /* register the APCS mux and divider clock */
> + ret = msm8916_register_clk(&pdev->dev, base);
Register a child platform device here instead of creating clks in the
same driver?
> + if (ret)
> + return ret;
> + }
> +
> offset = (unsigned long)of_device_get_match_data(&pdev->dev);
>
> apcs->reg = base + offset;
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On 26 June 2017 at 22:40, Rob Herring <[email protected]> wrote:
> On Fri, Jun 23, 2017 at 07:15:31PM +0300, 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 <[email protected]>
>> ---
>> .../devicetree/bindings/clock/qcom,a53pll.txt | 22 +++++
>
> It's preferred to split bindings to a separate patch. In any case,
>
> Acked-by: Rob Herring <[email protected]>
>
> One kconfig comment though:
>
>> drivers/clk/qcom/Kconfig | 9 +++
>> drivers/clk/qcom/Makefile | 1 +
>> drivers/clk/qcom/a53-pll.c | 94 ++++++++++++++++++++++
>> 4 files changed, 126 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..f4c2fddf6e7f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/qcom,a53pll.txt
>> @@ -0,0 +1,22 @@
>> +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..057cf60ed037 100644
>> --- a/drivers/clk/qcom/Kconfig
>> +++ b/drivers/clk/qcom/Kconfig
>> @@ -12,6 +12,15 @@ config COMMON_CLK_QCOM
>> select REGMAP_MMIO
>> select RESET_CONTROLLER
>>
>> +config QCOM_A53PLL
>> + bool "A53 PLL"
>
> Figuring out config options needed for a specific QC SoC is "fun". If
> this is only for MSM8916, then add that to the config option or prompt
> text at least.
> And please update the arm64 defconfig with this option.
Maybe instead of updating defconfig set:
default ARCH_QCOM
I think the effect is roughly same but it makes the life of
distribution maintainers easier.
>> + depends on COMMON_CLK_QCOM
>> + help
>> + Support for the A53 PLL on Qualcomm MSM8916 devices. It provides
>> + support for CPU frequencies above 1GHz.
>> + Say Y if you want to support CPU frequency scaling on devices
>> + such as MSM8916.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Bjorn,
On 06/23/2017 08:45 PM, Bjorn Andersson wrote:
> On Fri 23 Jun 09:15 PDT 2017, Georgi Djakov wrote:
>
>> +static int msm8916_register_clk(struct device *dev, void __iomem *base)
>> +{
> [..]
>> + regmap = devm_regmap_init_mmio(dev, base, &a53cc_regmap_config);
>> + if (IS_ERR(regmap)) {
>> + ret = PTR_ERR(regmap);
>> + dev_err(dev, "failed to init regmap mmio: %d\n", ret);
>> + goto err;
>> + }
>
> I think it would be cleaner if you create the regmap in probe() and we
> use that throughout the driver - rather than using two different access
> mechanism.
Ok agree, will make sure its consistent.
>
>> +
>> + a53cc->clkr.regmap = regmap;
>> +
>> + 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(dev->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;
>> + }
>> +
>> + return 0;
>> +
>> +err:
>> + clk_notifier_unregister(pclk, &a53cc->clk_nb);
>> + return ret;
>> +}
>> +
>> static int qcom_apcs_ipc_probe(struct platform_device *pdev)
>> {
>> + struct device_node *np = pdev->dev.of_node;
>> struct qcom_apcs_ipc *apcs;
>> struct resource *res;
>> unsigned long offset;
>> @@ -63,6 +178,13 @@ static int qcom_apcs_ipc_probe(struct platform_device *pdev)
>> if (IS_ERR(base))
>> return PTR_ERR(base);
>>
>> + if (of_device_is_compatible(np, "qcom,msm8916-apcs-kpss-global")) {
>> + /* register the APCS mux and divider clock */
>> + ret = msm8916_register_clk(&pdev->dev, base);
>> + if (ret)
>> + return ret;
>> + }
>> +
>
> Don't you need to clean up anything in the below error path and in
> remove()?
Right, will take care of it.
>
>> offset = (unsigned long)of_device_get_match_data(&pdev->dev);
>>
>> apcs->reg = base + offset;
>
> Other than that I think this looks reasonable.
Thanks for your time!
BR,
Georgi
On 06/27/2017 12:48 PM, Riku Voipio wrote:
> On 26 June 2017 at 22:40, Rob Herring <[email protected]> wrote:
>> On Fri, Jun 23, 2017 at 07:15:31PM +0300, 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 <[email protected]>
>>> ---
>>> .../devicetree/bindings/clock/qcom,a53pll.txt | 22 +++++
>>
>> It's preferred to split bindings to a separate patch. In any case,
>>
>> Acked-by: Rob Herring <[email protected]>
>>
>> One kconfig comment though:
>>
>>> drivers/clk/qcom/Kconfig | 9 +++
>>> drivers/clk/qcom/Makefile | 1 +
>>> drivers/clk/qcom/a53-pll.c | 94 ++++++++++++++++++++++
>>> 4 files changed, 126 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..f4c2fddf6e7f
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/clock/qcom,a53pll.txt
>>> @@ -0,0 +1,22 @@
>>> +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..057cf60ed037 100644
>>> --- a/drivers/clk/qcom/Kconfig
>>> +++ b/drivers/clk/qcom/Kconfig
>>> @@ -12,6 +12,15 @@ config COMMON_CLK_QCOM
>>> select REGMAP_MMIO
>>> select RESET_CONTROLLER
>>>
>>> +config QCOM_A53PLL
>>> + bool "A53 PLL"
>>
>> Figuring out config options needed for a specific QC SoC is "fun". If
>> this is only for MSM8916, then add that to the config option or prompt
>> text at least.
>
>> And please update the arm64 defconfig with this option.
>
> Maybe instead of updating defconfig set:
>
> default ARCH_QCOM
>
> I think the effect is roughly same but it makes the life of
> distribution maintainers easier.>
Thanks for the ideas, Riku and Rob. It looks like currently its only
used in msm8916 (but i might be wrong), so i will add MSM8916 to the
prompt and rename it to MSM_A53PLL_8916 to be more consistent with the
rest of the options.
If there are no objections, i can add:
default ARCH_QCOM && ARM64
Thanks,
Georgi
>>> + depends on COMMON_CLK_QCOM
>>> + help
>>> + Support for the A53 PLL on Qualcomm MSM8916 devices. It provides
>>> + support for CPU frequencies above 1GHz.
>>> + Say Y if you want to support CPU frequency scaling on devices
>>> + such as MSM8916.
Hi Stephen,
On 06/27/2017 01:47 AM, Stephen Boyd wrote:
> On 06/23, Georgi Djakov wrote:
>> diff --git a/drivers/mailbox/qcom-apcs-ipc-mailbox.c b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
>> index 9924c6d7f05d..da363c6580da 100644
>> --- a/drivers/mailbox/qcom-apcs-ipc-mailbox.c
>> +++ b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
>> @@ -11,6 +11,8 @@
>> * GNU General Public License for more details.
>> */
>>
>> +#include <linux/clk.h>
>> +#include <linux/clk-provider.h>
>> #include <linux/kernel.h>
>> #include <linux/module.h>
>> #include <linux/io.h>
>> @@ -19,6 +21,34 @@
>> #include <linux/of_platform.h>
>> #include <linux/platform_device.h>
>> #include <linux/mailbox_controller.h>
>> +#include <linux/regmap.h>
>> +
>> +#include "../clk/qcom/clk-regmap.h"
>> +#include "../clk/qcom/clk-regmap-mux-div.h"
>
> Why?
>
>> +
>> +
>> +static int msm8916_register_clk(struct device *dev, void __iomem *base)
>> +{
>> + struct clk_regmap_mux_div *a53cc;
>> + struct clk *pclk;
>> + struct regmap *regmap;
>> + struct clk_init_data init = { };
>> + int ret;
>> +
>> + a53cc = devm_kzalloc(dev, sizeof(*a53cc), GFP_KERNEL);
>> + if (!a53cc)
>> + return -ENOMEM;
>> +
>> + 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;
>> +
>> + init.name = "a53mux";
>> + init.parent_names = gpll0_a53cc;
>> + init.num_parents = 2;
>
> ARRAY_SIZE(gpll0_a53cc) instead of 2 please
Ok.
>
>> + init.ops = &clk_regmap_mux_div_ops;
>> + init.flags = CLK_SET_RATE_PARENT;
>> + a53cc->clkr.hw.init = &init;
>> +
>> + pclk = __clk_lookup(gpll0_a53cc[1]);
>
> Urgh.. ok. We can't clk_get()?
Ok, will do.
>
>> + if (!pclk)
>> + return -EPROBE_DEFER;
>> +
>> + a53cc->clk_nb.notifier_call = a53cc_notifier_cb;
>> + ret = clk_notifier_register(pclk, &a53cc->clk_nb);
>> + if (ret) {
>> + dev_err(dev, "failed to register clock notifier: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + regmap = devm_regmap_init_mmio(dev, base, &a53cc_regmap_config);
>> + if (IS_ERR(regmap)) {
>> + ret = PTR_ERR(regmap);
>> + dev_err(dev, "failed to init regmap mmio: %d\n", ret);
>> + goto err;
>> + }
>> +
>> + a53cc->clkr.regmap = regmap;
>> +
>> + ret = devm_clk_register_regmap(dev, &a53cc->clkr);
>
> Regmap is not a requirement to work with the qcom clk driver.
>
The other option then is to drop the regmap-mux-div patch and switch
to readl()/writel().
>> + if (ret) {
>> + dev_err(dev, "failed to register regmap clock: %d\n", ret);
>> + goto err;
>> + }
>> +
>> + ret = of_clk_add_hw_provider(dev->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;
>> + }
>> +
>> + return 0;
>> +
>> +err:
>> + clk_notifier_unregister(pclk, &a53cc->clk_nb);
>> + return ret;
>> +}
>> +
>> static int qcom_apcs_ipc_probe(struct platform_device *pdev)
>> {
>> + struct device_node *np = pdev->dev.of_node;
>> struct qcom_apcs_ipc *apcs;
>> struct resource *res;
>> unsigned long offset;
>> @@ -63,6 +178,13 @@ static int qcom_apcs_ipc_probe(struct platform_device *pdev)
>> if (IS_ERR(base))
>> return PTR_ERR(base);
>>
>> + if (of_device_is_compatible(np, "qcom,msm8916-apcs-kpss-global")) {
>> + /* register the APCS mux and divider clock */
>> + ret = msm8916_register_clk(&pdev->dev, base);
>
> Register a child platform device here instead of creating clks in the
> same driver?
Ok, we can register a child platform device and create a separate driver
in drivers/clk. If we are dropping regmap, then we can also move the
mux-div clk ops and clk registration into the same file. I will give it
a try.
Thanks,
Georgi
>
>> + if (ret)
>> + return ret;
>> + }
>> +
>> offset = (unsigned long)of_device_get_match_data(&pdev->dev);
>>
>> apcs->reg = base + offset;
>