[PATCH v2 0/2] Add initial support for RPM clocks

[PATCH v2 0/2] Add initial support for RPM clocks

[Georgi Djakov]

This patchset adds initial support for the clocks controlled by
the RPM (Resource Power Manager) processor on Qualcomm platforms.
It depends on Bjorn's Qualcomm SMD & RPM patches, that are now in
linux-next: https://lkml.org/lkml/2015/7/27/1125

The first patch implements the RPM clock operations, which are
using a shared memory for sending requests to the RPM processor.
The second patch adds the support of RPM clocks on the MSM8916
platform.

Changes since v1 (https://lkml.org/lkml/2015/7/9/257):
* Changed the driver name to clk-smd-rpm, also build it only
  when it is needed - suggested by Srini and Bjorn.
* More detailed binding example.
* Minor changes.

Georgi Djakov (2):
  clk: qcom: Add support for RPM Clocks
  clk: qcom: Add MSM8916 RPM clock driver

 .../devicetree/bindings/clock/qcom,rpmcc.txt       |   35 ++++
 drivers/clk/qcom/Kconfig                           |    4 +
 drivers/clk/qcom/Makefile                          |    2 +
 drivers/clk/qcom/clk-smd-rpm.c                     |  165 ++++++++++++++++
 drivers/clk/qcom/clk-smd-rpm.h                     |  139 ++++++++++++++
 drivers/clk/qcom/gcc-msm8916.c                     |   13 --
 drivers/clk/qcom/rpmcc-msm8916.c                   |  196 ++++++++++++++++++++
 include/dt-bindings/clock/qcom,rpmcc-msm8916.h     |   44 +++++
 8 files changed, 585 insertions(+), 13 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,rpmcc.txt
 create mode 100644 drivers/clk/qcom/clk-smd-rpm.c
 create mode 100644 drivers/clk/qcom/clk-smd-rpm.h
 create mode 100644 drivers/clk/qcom/rpmcc-msm8916.c
 create mode 100644 include/dt-bindings/clock/qcom,rpmcc-msm8916.h

[PATCH v2 1/2] clk: qcom: Add support for RPM Clocks

[Georgi Djakov]

This adds initial support for clocks controlled by the Resource
Power Manager (RPM) processor found on some Qualcomm SoCs.

The RPM is a dedicated hardware engine for managing the shared
SoC resources in order to keep the lowest power profile. It
communicates with other hardware subsystems via shared memory
and accepts clock requests, aggregates the requests and turns
the clocks on/off or scales them on demand.

This driver is based on the codeaurora.org driver:
https://www.codeaurora.org/cgit/quic/la/kernel/msm-3.10/tree/drivers/clk/qcom/clock-rpm.c

Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
---
 drivers/clk/qcom/Kconfig       |    3 +
 drivers/clk/qcom/Makefile      |    1 +
 drivers/clk/qcom/clk-smd-rpm.c |  165 ++++++++++++++++++++++++++++++++++++++++
 drivers/clk/qcom/clk-smd-rpm.h |  139 +++++++++++++++++++++++++++++++++
 4 files changed, 308 insertions(+)
 create mode 100644 drivers/clk/qcom/clk-smd-rpm.c
 create mode 100644 drivers/clk/qcom/clk-smd-rpm.h

diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index 59d16668bdf5..e347b97aa9c7 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -5,6 +5,9 @@ config COMMON_CLK_QCOM
 	select REGMAP_MMIO
 	select RESET_CONTROLLER
 
+config QCOM_CLK_SMD_RPM
+	bool
+
 config APQ_GCC_8084
 	tristate "APQ8084 Global Clock Controller"
 	depends on COMMON_CLK_QCOM
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index 50b337a24a87..33adf1d97da3 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -9,6 +9,7 @@ clk-qcom-y += clk-branch.o
 clk-qcom-y += clk-regmap-divider.o
 clk-qcom-y += clk-regmap-mux.o
 clk-qcom-y += reset.o
+clk-qcom-$(CONFIG_QCOM_CLK_SMD_RPM) += clk-smd-rpm.o
 
 obj-$(CONFIG_APQ_GCC_8084) += gcc-apq8084.o
 obj-$(CONFIG_APQ_MMCC_8084) += mmcc-apq8084.o
diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
new file mode 100644
index 000000000000..e564673ec3a5
--- /dev/null
+++ b/drivers/clk/qcom/clk-smd-rpm.c
@@ -0,0 +1,165 @@
+/*
+ * 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.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/export.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+
+#include "clk-smd-rpm.h"
+
+static int clk_smd_rpm_set_rate_active(struct clk_smd_rpm *r,
+				       unsigned long value)
+{
+	struct clk_smd_rpm_req req = {
+		.key = QCOM_RPM_SMD_KEY_RATE,
+		.nbytes = sizeof(u32),
+		.value = DIV_ROUND_UP(value, 1000), /* RPM expects KHz */
+	};
+
+	return qcom_rpm_smd_write(r->rpm, QCOM_SMD_RPM_ACTIVE_STATE,
+				  r->rpm_res_type, r->rpm_clk_id, &req,
+				  sizeof(req));
+}
+
+static int clk_smd_rpm_prepare(struct clk_hw *hw)
+{
+	struct clk_smd_rpm *r = to_clk_smd_rpm(hw);
+	struct clk_smd_rpm *peer = r->peer;
+	u32 value;
+	int ret = 0;
+
+	/* Don't send requests to the RPM if the rate has not been set. */
+	if (!r->rate)
+		goto out;
+
+	/* Take peer clock's rate into account only if it's enabled. */
+	if (peer->enabled)
+		value = max(r->rate, peer->rate);
+	else
+		value = r->rate;
+
+	if (r->branch)
+		value = !!value;
+
+	ret = clk_smd_rpm_set_rate_active(r, value);
+	if (ret)
+		goto out;
+
+out:
+	if (!ret)
+		r->enabled = true;
+
+	return ret;
+}
+
+static void clk_smd_rpm_unprepare(struct clk_hw *hw)
+{
+	struct clk_smd_rpm *r = to_clk_smd_rpm(hw);
+
+	if (r->rate) {
+		struct clk_smd_rpm *peer = r->peer;
+		unsigned long peer_rate;
+		u32 value;
+		int ret;
+
+		/* Take peer clock's rate into account only if it's enabled. */
+		peer_rate = peer->enabled ? peer->rate : 0;
+		value = r->branch ? !!peer_rate : peer_rate;
+		ret = clk_smd_rpm_set_rate_active(r, value);
+		if (ret)
+			return;
+	}
+	r->enabled = false;
+}
+
+static int clk_smd_rpm_set_rate(struct clk_hw *hw, unsigned long rate,
+				unsigned long parent_rate)
+{
+	struct clk_smd_rpm *r = to_clk_smd_rpm(hw);
+	int ret = 0;
+
+	if (r->enabled) {
+		u32 value;
+		struct clk_smd_rpm *peer = r->peer;
+
+		/* Take peer clock's rate into account only if it's enabled. */
+		if (peer->enabled)
+			value = max(rate, peer->rate);
+		else
+			value = rate;
+
+		ret = clk_smd_rpm_set_rate_active(r, value);
+		if (ret)
+			goto out;
+	}
+	r->rate = rate;
+out:
+	return ret;
+}
+
+static long clk_smd_rpm_round_rate(struct clk_hw *hw, unsigned long rate,
+				   unsigned long *parent_rate)
+{
+	return rate;
+}
+
+static unsigned long clk_smd_rpm_recalc_rate(struct clk_hw *hw,
+					     unsigned long parent_rate)
+{
+	struct clk_smd_rpm *r = to_clk_smd_rpm(hw);
+
+	return r->rate;
+}
+
+int clk_smd_rpm_enable_scaling(struct qcom_smd_rpm *rpm)
+{
+	int ret;
+	struct clk_smd_rpm_req req = {
+		.key = QCOM_RPM_SMD_KEY_ENABLE,
+		.nbytes = sizeof(u32),
+		.value = 1,
+	};
+
+	ret = qcom_rpm_smd_write(rpm, QCOM_SMD_RPM_ACTIVE_STATE,
+				 QCOM_SMD_RPM_MISC_CLK,
+				 QCOM_RPM_SCALING_ENABLE_ID, &req, sizeof(req));
+	if (ret < 0) {
+		if (ret != -EPROBE_DEFER)
+			pr_err("RPM clock scaling (active set) not enabled!\n");
+		return ret;
+	}
+
+	pr_debug("%s: RPM clock scaling is enabled\n", __func__);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(clk_smd_rpm_enable_scaling);
+
+const struct clk_ops clk_smd_rpm_ops = {
+	.prepare = clk_smd_rpm_prepare,
+	.unprepare = clk_smd_rpm_unprepare,
+	.set_rate = clk_smd_rpm_set_rate,
+	.round_rate = clk_smd_rpm_round_rate,
+	.recalc_rate = clk_smd_rpm_recalc_rate,
+};
+EXPORT_SYMBOL_GPL(clk_smd_rpm_ops);
+
+const struct clk_ops clk_smd_rpm_branch_ops = {
+	.prepare = clk_smd_rpm_prepare,
+	.unprepare = clk_smd_rpm_unprepare,
+	.round_rate = clk_smd_rpm_round_rate,
+	.recalc_rate = clk_smd_rpm_recalc_rate,
+};
+EXPORT_SYMBOL_GPL(clk_smd_rpm_branch_ops);
diff --git a/drivers/clk/qcom/clk-smd-rpm.h b/drivers/clk/qcom/clk-smd-rpm.h
new file mode 100644
index 000000000000..7fd67c0e31b5
--- /dev/null
+++ b/drivers/clk/qcom/clk-smd-rpm.h
@@ -0,0 +1,139 @@
+/*
+ * 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_SMD_RPM_H__
+#define __QCOM_CLK_SMD_RPM_H__
+
+#include <linux/clk-provider.h>
+#include <linux/soc/qcom/smd-rpm.h>
+
+#define QCOM_RPM_KEY_SOFTWARE_ENABLE			0x6e657773
+#define QCOM_RPM_KEY_PIN_CTRL_CLK_BUFFER_ENABLE_KEY	0x62636370
+#define QCOM_RPM_SMD_KEY_RATE				0x007a484b
+#define QCOM_RPM_SMD_KEY_ENABLE				0x62616e45
+#define QCOM_RPM_SMD_KEY_STATE				0x54415453
+#define QCOM_RPM_SCALING_ENABLE_ID			0x2
+
+struct clk_smd_rpm {
+	const int rpm_res_type;
+	const int rpm_key;
+	const int rpm_clk_id;
+	const int rpm_status_id;
+	const bool active_only;
+	bool enabled;
+	bool branch;
+	struct clk_smd_rpm *peer;
+	struct clk_hw hw;
+	unsigned long rate;
+	struct qcom_smd_rpm *rpm;
+};
+
+struct clk_smd_rpm_req {
+	u32 key;
+	u32 nbytes;
+	u32 value;
+};
+
+extern const struct clk_ops clk_smd_rpm_ops;
+extern const struct clk_ops clk_smd_rpm_branch_ops;
+int clk_smd_rpm_enable_scaling(struct qcom_smd_rpm *rpm);
+
+#define to_clk_smd_rpm(_hw) container_of(_hw, struct clk_smd_rpm, hw)
+
+#define __DEFINE_CLK_SMD_RPM(_name, active, type, r_id, stat_id, dep, key) \
+	static struct clk_smd_rpm active; \
+	static struct clk_smd_rpm _name = { \
+		.rpm_res_type = (type), \
+		.rpm_clk_id = (r_id), \
+		.rpm_status_id = (stat_id), \
+		.rpm_key = (key), \
+		.peer = &active, \
+		.rate = INT_MAX, \
+		.hw.init = &(struct clk_init_data){ \
+			.ops = &clk_smd_rpm_ops, \
+			.name = #_name, \
+			.flags = CLK_IS_ROOT, \
+		}, \
+	}; \
+	static struct clk_smd_rpm active = { \
+		.rpm_res_type = (type), \
+		.rpm_clk_id = (r_id), \
+		.rpm_status_id = (stat_id), \
+		.rpm_key = (key), \
+		.peer = &_name, \
+		.active_only = true, \
+		.rate = INT_MAX, \
+		.hw.init = &(struct clk_init_data){ \
+			.ops = &clk_smd_rpm_ops, \
+			.name = #active, \
+			.flags = CLK_IS_ROOT, \
+		}, \
+	};
+
+#define __DEFINE_CLK_SMD_RPM_BRANCH(_name, active, type, r_id, stat_id, r, \
+				key) \
+	static struct clk_smd_rpm active; \
+	static struct clk_smd_rpm _name = { \
+		.rpm_res_type = (type), \
+		.rpm_clk_id = (r_id), \
+		.rpm_status_id = (stat_id), \
+		.rpm_key = (key), \
+		.peer = &active, \
+		.branch = true, \
+		.rate = (r), \
+		.hw.init = &(struct clk_init_data){ \
+			.ops = &clk_smd_rpm_branch_ops, \
+			.name = #_name, \
+			.flags = CLK_IS_ROOT, \
+		}, \
+	}; \
+	static struct clk_smd_rpm active = { \
+		.rpm_res_type = (type), \
+		.rpm_clk_id = (r_id), \
+		.rpm_status_id = (stat_id), \
+		.rpm_key = (key), \
+		.peer = &_name, \
+		.active_only = true, \
+		.branch = true, \
+		.rate = (r), \
+		.hw.init = &(struct clk_init_data){ \
+			.ops = &clk_smd_rpm_branch_ops, \
+			.name = #active, \
+			.flags = CLK_IS_ROOT, \
+		}, \
+	};
+
+#define DEFINE_CLK_SMD_RPM(_name, active, type, r_id, dep) \
+		__DEFINE_CLK_SMD_RPM(_name, active, type, r_id, 0, dep, \
+		QCOM_RPM_SMD_KEY_RATE)
+
+#define DEFINE_CLK_SMD_RPM_BRANCH(_name, active, type, r_id, r) \
+		__DEFINE_CLK_SMD_RPM_BRANCH(_name, active, type, r_id, 0, r, \
+		QCOM_RPM_SMD_KEY_ENABLE)
+
+#define DEFINE_CLK_SMD_RPM_QDSS(_name, active, type, r_id) \
+		__DEFINE_CLK_SMD_RPM(_name, active, type, r_id, \
+		0, 0, QCOM_RPM_SMD_KEY_STATE)
+
+#define DEFINE_CLK_SMD_RPM_XO_BUFFER(_name, active, r_id) \
+		__DEFINE_CLK_SMD_RPM_BRANCH(_name, active, \
+		QCOM_SMD_RPM_CLK_BUF_A, r_id, 0, 1000, \
+		QCOM_RPM_KEY_SOFTWARE_ENABLE)
+
+#define DEFINE_CLK_SMD_RPM_XO_BUFFER_PINCTRL(_name, active, r_id) \
+		__DEFINE_CLK_SMD_RPM_BRANCH(_name, active, \
+		QCOM_SMD_RPM_CLK_BUF_A, r_id, 0, 1000, \
+		QCOM_RPM_KEY_PIN_CTRL_CLK_BUFFER_ENABLE_KEY)
+
+#endif

Re: [PATCH v2 1/2] clk: qcom: Add support for RPM Clocks

[Stephen Boyd]

On 08/03, Georgi Djakov wrote:
> diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
> new file mode 100644
> index 000000000000..e564673ec3a5
> --- /dev/null
> +++ b/drivers/clk/qcom/clk-smd-rpm.c
> @@ -0,0 +1,165 @@
> +/*
> + * 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.
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/err.h>
> +#include <linux/export.h>
> +#include <linux/kernel.h>
> +#include <linux/platform_device.h>

Is this include used?

> +
> +#include "clk-smd-rpm.h"
> +
> +static int clk_smd_rpm_set_rate_active(struct clk_smd_rpm *r,
> +				       unsigned long value)
>y +{
> +	struct clk_smd_rpm_req req = {
> +		.key = QCOM_RPM_SMD_KEY_RATE,
> +		.nbytes = sizeof(u32),
> +		.value = DIV_ROUND_UP(value, 1000), /* RPM expects KHz */

s/KHz/kHz/

> +	};
> +
> +	return qcom_rpm_smd_write(r->rpm, QCOM_SMD_RPM_ACTIVE_STATE,
> +				  r->rpm_res_type, r->rpm_clk_id, &req,
> +				  sizeof(req));
> +}
> +
> +static int clk_smd_rpm_prepare(struct clk_hw *hw)
> +{
> +	struct clk_smd_rpm *r = to_clk_smd_rpm(hw);
> +	struct clk_smd_rpm *peer = r->peer;
> +	u32 value;
> +	int ret = 0;
> +
> +	/* Don't send requests to the RPM if the rate has not been set. */
> +	if (!r->rate)
> +		goto out;
> +
> +	/* Take peer clock's rate into account only if it's enabled. */
> +	if (peer->enabled)

We need some sort of lock here. Please make an internal mutex
like the downstream code to protect accesses from one RPM clock
to another RPM clock.

> +		value = max(r->rate, peer->rate);
> +	else
> +		value = r->rate;
> +
> +	if (r->branch)
> +		value = !!value;
> +
> +	ret = clk_smd_rpm_set_rate_active(r, value);
> +	if (ret)
> +		goto out;
> +
> +out:
> +	if (!ret)
> +		r->enabled = true;
> +
> +	return ret;
> +}
> +
> +static void clk_smd_rpm_unprepare(struct clk_hw *hw)
> +{
> +	struct clk_smd_rpm *r = to_clk_smd_rpm(hw);
> +
> +	if (r->rate) {
> +		struct clk_smd_rpm *peer = r->peer;
> +		unsigned long peer_rate;
> +		u32 value;

Why not unsigned long?

> +		int ret;
> +
> +		/* Take peer clock's rate into account only if it's enabled. */
> +		peer_rate = peer->enabled ? peer->rate : 0;
> +		value = r->branch ? !!peer_rate : peer_rate;
> +		ret = clk_smd_rpm_set_rate_active(r, value);
> +		if (ret)
> +			return;
> +	}
> +	r->enabled = false;
> +}
> +
> +static int clk_smd_rpm_set_rate(struct clk_hw *hw, unsigned long rate,
> +				unsigned long parent_rate)
> +{
> +	struct clk_smd_rpm *r = to_clk_smd_rpm(hw);
> +	int ret = 0;
> +
> +	if (r->enabled) {
> +		u32 value;
> +		struct clk_smd_rpm *peer = r->peer;
> +
> +		/* Take peer clock's rate into account only if it's enabled. */
> +		if (peer->enabled)

This peer stuff almost doesn't even matter because we're only
sending active set requests. Why can't this code be updated to
send both active and sleep set requests? The sleep set stuff
won't be cached, etc., but I don't see a problem in doing both.
Otherwise we should drop all the peer stuff until we introduce
active only clocks.

> +			value = max(rate, peer->rate);
> +		else
> +			value = rate;
> +
> +		ret = clk_smd_rpm_set_rate_active(r, value);
> +		if (ret)
> +			goto out;
> +	}
> +	r->rate = rate;
> +out:
> +	return ret;
> +}
> +
> +static long clk_smd_rpm_round_rate(struct clk_hw *hw, unsigned long rate,
> +				   unsigned long *parent_rate)
> +{

Please add a comment here like

	/*
 	 * RPM handles rate rounding and we don't have a way to
	 * know what the rate will be, so just return whatever
	 * rate is requested.
	 */

> +	return rate;
> +}
> +
> +static unsigned long clk_smd_rpm_recalc_rate(struct clk_hw *hw,
> +					     unsigned long parent_rate)
> +{
> +	struct clk_smd_rpm *r = to_clk_smd_rpm(hw);

And something similar here...

	/*
 	 * RPM handles rate rounding and we don't have a way to
	 * know what the rate will be, so just return whatever
	 * rate was set.
	 */

> +
> +	return r->rate;
> +}
> +
> +int clk_smd_rpm_enable_scaling(struct qcom_smd_rpm *rpm)
> +{
> +	int ret;
> +	struct clk_smd_rpm_req req = {
> +		.key = QCOM_RPM_SMD_KEY_ENABLE,
> +		.nbytes = sizeof(u32),
> +		.value = 1,
> +	};
> +
> +	ret = qcom_rpm_smd_write(rpm, QCOM_SMD_RPM_ACTIVE_STATE,
> +				 QCOM_SMD_RPM_MISC_CLK,
> +				 QCOM_RPM_SCALING_ENABLE_ID, &req, sizeof(req));
> +	if (ret < 0) {
> +		if (ret != -EPROBE_DEFER)

Does this API return EPROBE_DEFER?

> +			pr_err("RPM clock scaling (active set) not enabled!\n");
> +		return ret;
> +	}
> +
> +	pr_debug("%s: RPM clock scaling is enabled\n", __func__);
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(clk_smd_rpm_enable_scaling);
> diff --git a/drivers/clk/qcom/clk-smd-rpm.h b/drivers/clk/qcom/clk-smd-rpm.h
> new file mode 100644
> index 000000000000..7fd67c0e31b5
> --- /dev/null
> +++ b/drivers/clk/qcom/clk-smd-rpm.h
> @@ -0,0 +1,139 @@
> +/*
> + * 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_SMD_RPM_H__
> +#define __QCOM_CLK_SMD_RPM_H__
> +
> +#include <linux/clk-provider.h>
> +#include <linux/soc/qcom/smd-rpm.h>

Drop this include and forward declare struct qcom_smd_rpm?

> +
> +#define QCOM_RPM_KEY_SOFTWARE_ENABLE			0x6e657773
> +#define QCOM_RPM_KEY_PIN_CTRL_CLK_BUFFER_ENABLE_KEY	0x62636370
> +#define QCOM_RPM_SMD_KEY_RATE				0x007a484b
> +#define QCOM_RPM_SMD_KEY_ENABLE				0x62616e45
> +#define QCOM_RPM_SMD_KEY_STATE				0x54415453
> +#define QCOM_RPM_SCALING_ENABLE_ID			0x2
> +
> +struct clk_smd_rpm {
> +	const int rpm_res_type;
> +	const int rpm_key;
> +	const int rpm_clk_id;
> +	const int rpm_status_id;
> +	const bool active_only;
> +	bool enabled;
> +	bool branch;
> +	struct clk_smd_rpm *peer;
> +	struct clk_hw hw;
> +	unsigned long rate;
> +	struct qcom_smd_rpm *rpm;
> +};
> +
> +struct clk_smd_rpm_req {
> +	u32 key;
> +	u32 nbytes;
> +	u32 value;

Should all be __le32.

> +};
> +
> +extern const struct clk_ops clk_smd_rpm_ops;
> +extern const struct clk_ops clk_smd_rpm_branch_ops;
> +int clk_smd_rpm_enable_scaling(struct qcom_smd_rpm *rpm);
> +
> +#define to_clk_smd_rpm(_hw) container_of(_hw, struct clk_smd_rpm, hw)

Can we move this to the C file? We shouldn't need to use this
outside of the file that implements the ops.


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

Re: [PATCH v2 1/2] clk: qcom: Add support for RPM Clocks

[Georgi Djakov]

Hi Stephen,

On 09/02/2015 11:31 PM, Stephen Boyd wrote:
> On 08/03, Georgi Djakov wrote:
>> diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
>> new file mode 100644
>> index 000000000000..e564673ec3a5
>> --- /dev/null
>> +++ b/drivers/clk/qcom/clk-smd-rpm.c

[..]

>> +static int clk_smd_rpm_set_rate(struct clk_hw *hw, unsigned long rate,
>> +				unsigned long parent_rate)
>> +{
>> +	struct clk_smd_rpm *r = to_clk_smd_rpm(hw);
>> +	int ret = 0;
>> +
>> +	if (r->enabled) {
>> +		u32 value;
>> +		struct clk_smd_rpm *peer = r->peer;
>> +
>> +		/* Take peer clock's rate into account only if it's enabled. */
>> +		if (peer->enabled)
> 
> This peer stuff almost doesn't even matter because we're only
> sending active set requests. Why can't this code be updated to
> send both active and sleep set requests? The sleep set stuff
> won't be cached, etc., but I don't see a problem in doing both.
> Otherwise we should drop all the peer stuff until we introduce
> active only clocks.

Initially I tried sending both active and sleep sets, but as they are
not cached like in downstream (yet) i got hangs during boot. Disabling
caching in downstream kernel also caused the same hangs, so i left
this out for now. Will try debugging it further.

Will fix the rest according to your comments. Thank you!

BR,
Georgi

Re: [PATCH v2 1/2] clk: qcom: Add support for RPM Clocks

[Bjorn Andersson]

On Thu 03 Sep 08:40 PDT 2015, Georgi Djakov wrote:

> Hi Stephen,
> 
> On 09/02/2015 11:31 PM, Stephen Boyd wrote:
> > On 08/03, Georgi Djakov wrote:
> >> diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
> >> new file mode 100644
> >> index 000000000000..e564673ec3a5
> >> --- /dev/null
> >> +++ b/drivers/clk/qcom/clk-smd-rpm.c
> 
> [..]
> 
> >> +static int clk_smd_rpm_set_rate(struct clk_hw *hw, unsigned long rate,
> >> +				unsigned long parent_rate)
> >> +{
> >> +	struct clk_smd_rpm *r = to_clk_smd_rpm(hw);
> >> +	int ret = 0;
> >> +
> >> +	if (r->enabled) {
> >> +		u32 value;
> >> +		struct clk_smd_rpm *peer = r->peer;
> >> +
> >> +		/* Take peer clock's rate into account only if it's enabled. */
> >> +		if (peer->enabled)
> > 
> > This peer stuff almost doesn't even matter because we're only
> > sending active set requests. Why can't this code be updated to
> > send both active and sleep set requests? The sleep set stuff
> > won't be cached, etc., but I don't see a problem in doing both.
> > Otherwise we should drop all the peer stuff until we introduce
> > active only clocks.
> 
> Initially I tried sending both active and sleep sets, but as they are
> not cached like in downstream (yet) i got hangs during boot. Disabling
> caching in downstream kernel also caused the same hangs, so i left
> this out for now. Will try debugging it further.
> 

This sounds odd, although I presume the downstream code is rarely/never
tested with the caching disabled.

Can you please retry this with [1] applied (should be in -next), the RPM
fifo is tiny, so I would not be surprised if this could be your problem.

[1] https://lkml.org/lkml/2015/8/24/756

Regards,
Bjorn

Re: [PATCH v2 1/2] clk: qcom: Add support for RPM Clocks

[Bjorn Andersson]

On Thu 03 Sep 08:40 PDT 2015, Georgi Djakov wrote:

> Hi Stephen,
> 
> On 09/02/2015 11:31 PM, Stephen Boyd wrote:
> > On 08/03, Georgi Djakov wrote:
> >> diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
> >> new file mode 100644
> >> index 000000000000..e564673ec3a5
> >> --- /dev/null
> >> +++ b/drivers/clk/qcom/clk-smd-rpm.c
> 
> [..]
> 
> >> +static int clk_smd_rpm_set_rate(struct clk_hw *hw, unsigned long rate,
> >> +				unsigned long parent_rate)
> >> +{
> >> +	struct clk_smd_rpm *r = to_clk_smd_rpm(hw);
> >> +	int ret = 0;
> >> +
> >> +	if (r->enabled) {
> >> +		u32 value;
> >> +		struct clk_smd_rpm *peer = r->peer;
> >> +
> >> +		/* Take peer clock's rate into account only if it's enabled. */
> >> +		if (peer->enabled)
> > 
> > This peer stuff almost doesn't even matter because we're only
> > sending active set requests. Why can't this code be updated to
> > send both active and sleep set requests? The sleep set stuff
> > won't be cached, etc., but I don't see a problem in doing both.
> > Otherwise we should drop all the peer stuff until we introduce
> > active only clocks.
> 
> Initially I tried sending both active and sleep sets, but as they are
> not cached like in downstream (yet) i got hangs during boot. Disabling
> caching in downstream kernel also caused the same hangs, so i left
> this out for now. Will try debugging it further.
> 

This sounds odd, although I presume the downstream code is rarely/never
tested with the caching disabled.

Can you please retry this with [1] applied (should be in -next), the RPM
fifo is tiny, so I would not be surprised if this could be your problem.

[1] https://lkml.org/lkml/2015/8/24/756

Regards,
Bjorn

Re: [PATCH v2 1/2] clk: qcom: Add support for RPM Clocks

[Stephen Boyd]

On 09/03, Georgi Djakov wrote:
> Hi Stephen,
> 
> On 09/02/2015 11:31 PM, Stephen Boyd wrote:
> > On 08/03, Georgi Djakov wrote:
> >> diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
> >> new file mode 100644
> >> index 000000000000..e564673ec3a5
> >> --- /dev/null
> >> +++ b/drivers/clk/qcom/clk-smd-rpm.c
> 
> [..]
> 
> >> +static int clk_smd_rpm_set_rate(struct clk_hw *hw, unsigned long rate,
> >> +				unsigned long parent_rate)
> >> +{
> >> +	struct clk_smd_rpm *r = to_clk_smd_rpm(hw);
> >> +	int ret = 0;
> >> +
> >> +	if (r->enabled) {
> >> +		u32 value;
> >> +		struct clk_smd_rpm *peer = r->peer;
> >> +
> >> +		/* Take peer clock's rate into account only if it's enabled. */
> >> +		if (peer->enabled)
> > 
> > This peer stuff almost doesn't even matter because we're only
> > sending active set requests. Why can't this code be updated to
> > send both active and sleep set requests? The sleep set stuff
> > won't be cached, etc., but I don't see a problem in doing both.
> > Otherwise we should drop all the peer stuff until we introduce
> > active only clocks.
> 
> Initially I tried sending both active and sleep sets, but as they are
> not cached like in downstream (yet) i got hangs during boot. Disabling
> caching in downstream kernel also caused the same hangs, so i left
> this out for now. Will try debugging it further.

Ok. That's mildly concerning.

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

[PATCH v2 2/2] clk: qcom: Add MSM8916 RPM clock driver

[Georgi Djakov]

Add support for clocks that are controlled by the RPM processor
on Qualcomm msm8916 based platforms.

Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
---
 .../devicetree/bindings/clock/qcom,rpmcc.txt       |   35 ++++
 drivers/clk/qcom/Kconfig                           |    1 +
 drivers/clk/qcom/Makefile                          |    1 +
 drivers/clk/qcom/gcc-msm8916.c                     |   13 --
 drivers/clk/qcom/rpmcc-msm8916.c                   |  196 ++++++++++++++++++++
 include/dt-bindings/clock/qcom,rpmcc-msm8916.h     |   44 +++++
 6 files changed, 277 insertions(+), 13 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,rpmcc.txt
 create mode 100644 drivers/clk/qcom/rpmcc-msm8916.c
 create mode 100644 include/dt-bindings/clock/qcom,rpmcc-msm8916.h

diff --git a/Documentation/devicetree/bindings/clock/qcom,rpmcc.txt b/Documentation/devicetree/bindings/clock/qcom,rpmcc.txt
new file mode 100644
index 000000000000..bd0fd0cd50dc
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/qcom,rpmcc.txt
@@ -0,0 +1,35 @@
+Qualcomm RPM Clock Controller Binding
+------------------------------------------------
+The RPM is a dedicated hardware engine for managing the shared
+SoC resources in order to keep the lowest power profile. It
+communicates with other hardware subsystems via shared memory
+and accepts clock requests, aggregates the requests and turns
+the clocks on/off or scales them on demand.
+
+Required properties :
+- compatible : shall contain only one of the following:
+
+			"qcom,rpmcc-msm8916"
+
+- #clock-cells : shall contain 1
+
+Example:
+	smd {
+		compatible = "qcom,smd";
+
+		rpm {
+			interrupts = <0 168 1>;
+			qcom,ipc = <&apcs 8 0>;
+			qcom,smd-edge = <15>;
+
+			rpm_requests {
+				compatible = "qcom,rpm-msm8916";
+				qcom,smd-channels = "rpm_requests";
+
+				rpmcc: qcom,rpmcc {
+					compatible = "qcom,rpmcc-msm8916";
+					#clock-cells = <1>;
+				};
+			};
+		};
+	};
diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index e347b97aa9c7..edffb57a3aff 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -52,6 +52,7 @@ config MSM_GCC_8660
 
 config MSM_GCC_8916
 	tristate "MSM8916 Global Clock Controller"
+	select QCOM_CLK_SMD_RPM
 	depends on COMMON_CLK_QCOM
 	help
 	  Support for the global clock controller on msm8916 devices.
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index 33adf1d97da3..985c794608a7 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_IPQ_GCC_806X) += gcc-ipq806x.o
 obj-$(CONFIG_IPQ_LCC_806X) += lcc-ipq806x.o
 obj-$(CONFIG_MSM_GCC_8660) += gcc-msm8660.o
 obj-$(CONFIG_MSM_GCC_8916) += gcc-msm8916.o
+obj-$(CONFIG_MSM_GCC_8916) += rpmcc-msm8916.o
 obj-$(CONFIG_MSM_GCC_8960) += gcc-msm8960.o
 obj-$(CONFIG_MSM_LCC_8960) += lcc-msm8960.o
 obj-$(CONFIG_MSM_GCC_8974) += gcc-msm8974.o
diff --git a/drivers/clk/qcom/gcc-msm8916.c b/drivers/clk/qcom/gcc-msm8916.c
index 3bf4fb3deef6..28ef2c771157 100644
--- a/drivers/clk/qcom/gcc-msm8916.c
+++ b/drivers/clk/qcom/gcc-msm8916.c
@@ -2820,19 +2820,6 @@ MODULE_DEVICE_TABLE(of, gcc_msm8916_match_table);
 
 static int gcc_msm8916_probe(struct platform_device *pdev)
 {
-	struct clk *clk;
-	struct device *dev = &pdev->dev;
-
-	/* Temporary until RPM clocks supported */
-	clk = clk_register_fixed_rate(dev, "xo", NULL, CLK_IS_ROOT, 19200000);
-	if (IS_ERR(clk))
-		return PTR_ERR(clk);
-
-	clk = clk_register_fixed_rate(dev, "sleep_clk_src", NULL,
-				      CLK_IS_ROOT, 32768);
-	if (IS_ERR(clk))
-		return PTR_ERR(clk);
-
 	return qcom_cc_probe(pdev, &gcc_msm8916_desc);
 }
 
diff --git a/drivers/clk/qcom/rpmcc-msm8916.c b/drivers/clk/qcom/rpmcc-msm8916.c
new file mode 100644
index 000000000000..60b2292dcd45
--- /dev/null
+++ b/drivers/clk/qcom/rpmcc-msm8916.c
@@ -0,0 +1,196 @@
+/*
+ * 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.
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+#include "clk-smd-rpm.h"
+#include <dt-bindings/clock/qcom,rpmcc-msm8916.h>
+
+#define CXO_ID			0x0
+#define QDSS_ID			0x1
+#define BUS_SCALING		0x2
+
+#define PCNOC_ID		0x0
+#define SNOC_ID			0x1
+#define BIMC_ID			0x0
+
+#define BB_CLK1_ID		0x1
+#define BB_CLK2_ID		0x2
+#define RF_CLK1_ID		0x4
+#define RF_CLK2_ID		0x5
+
+struct rpm_cc {
+	struct clk_onecell_data data;
+	struct clk *clks[];
+};
+
+/* SMD clocks */
+DEFINE_CLK_SMD_RPM(pcnoc_clk, pcnoc_a_clk, QCOM_SMD_RPM_BUS_CLK, PCNOC_ID, NULL);
+DEFINE_CLK_SMD_RPM(snoc_clk, snoc_a_clk, QCOM_SMD_RPM_BUS_CLK, SNOC_ID, NULL);
+DEFINE_CLK_SMD_RPM(bimc_clk, bimc_a_clk, QCOM_SMD_RPM_MEM_CLK, BIMC_ID, NULL);
+
+DEFINE_CLK_SMD_RPM_BRANCH(xo, xo_a, QCOM_SMD_RPM_MISC_CLK, CXO_ID, 19200000);
+DEFINE_CLK_SMD_RPM_QDSS(qdss_clk, qdss_a_clk, QCOM_SMD_RPM_MISC_CLK, QDSS_ID);
+
+/* SMD_XO_BUFFER */
+DEFINE_CLK_SMD_RPM_XO_BUFFER(bb_clk1, bb_clk1_a, BB_CLK1_ID);
+DEFINE_CLK_SMD_RPM_XO_BUFFER(bb_clk2, bb_clk2_a, BB_CLK2_ID);
+DEFINE_CLK_SMD_RPM_XO_BUFFER(rf_clk1, rf_clk1_a, RF_CLK1_ID);
+DEFINE_CLK_SMD_RPM_XO_BUFFER(rf_clk2, rf_clk2_a, RF_CLK2_ID);
+
+DEFINE_CLK_SMD_RPM_XO_BUFFER_PINCTRL(bb_clk1_pin, bb_clk1_a_pin, BB_CLK1_ID);
+DEFINE_CLK_SMD_RPM_XO_BUFFER_PINCTRL(bb_clk2_pin, bb_clk2_a_pin, BB_CLK2_ID);
+DEFINE_CLK_SMD_RPM_XO_BUFFER_PINCTRL(rf_clk1_pin, rf_clk1_a_pin, RF_CLK1_ID);
+DEFINE_CLK_SMD_RPM_XO_BUFFER_PINCTRL(rf_clk2_pin, rf_clk2_a_pin, RF_CLK2_ID);
+
+static struct clk_smd_rpm *rpmcc_msm8916_clks[] = {
+	[RPM_XO_CLK_SRC] = &xo,
+	[RPM_XO_A_CLK_SRC] = &xo_a,
+	[RPM_PCNOC_CLK] = &pcnoc_clk,
+	[RPM_PCNOC_A_CLK] = &pcnoc_a_clk,
+	[RPM_SNOC_CLK] = &snoc_clk,
+	[RPM_SNOC_A_CLK] = &snoc_a_clk,
+	[RPM_BIMC_CLK] = &bimc_clk,
+	[RPM_BIMC_A_CLK] = &bimc_a_clk,
+	[RPM_QDSS_CLK] = &qdss_clk,
+	[RPM_QDSS_A_CLK] = &qdss_a_clk,
+	[RPM_BB_CLK1] = &bb_clk1,
+	[RPM_BB_CLK1_A] = &bb_clk1_a,
+	[RPM_BB_CLK2] = &bb_clk2,
+	[RPM_BB_CLK2_A] = &bb_clk2_a,
+	[RPM_RF_CLK1] = &rf_clk1,
+	[RPM_RF_CLK1_A] = &rf_clk1_a,
+	[RPM_RF_CLK2] = &rf_clk2,
+	[RPM_RF_CLK2_A] = &rf_clk2_a,
+	[RPM_BB_CLK1_PIN] = &bb_clk1_pin,
+	[RPM_BB_CLK1_A_PIN] = &bb_clk1_a_pin,
+	[RPM_BB_CLK2_PIN] = &bb_clk2_pin,
+	[RPM_BB_CLK2_A_PIN] = &bb_clk2_a_pin,
+	[RPM_RF_CLK1_PIN] = &rf_clk1_pin,
+	[RPM_RF_CLK1_A_PIN] = &rf_clk1_a_pin,
+	[RPM_RF_CLK2_PIN] = &rf_clk2_pin,
+	[RPM_RF_CLK2_A_PIN] = &rf_clk2_a_pin,
+};
+
+static int rpmcc_msm8916_probe(struct platform_device *pdev)
+{
+	struct clk **clks;
+	struct clk *clk;
+	struct rpm_cc *rcc;
+	struct qcom_smd_rpm *rpm;
+	struct clk_onecell_data *data;
+	int num_clks = ARRAY_SIZE(rpmcc_msm8916_clks);
+	int ret, i;
+
+	if (!pdev->dev.of_node)
+		return -ENODEV;
+
+	rpm = dev_get_drvdata(pdev->dev.parent);
+	if (!rpm) {
+		dev_err(&pdev->dev, "Unable to retrieve handle to RPM\n");
+		return -ENODEV;
+	}
+
+	ret = clk_smd_rpm_enable_scaling(rpm);
+	if (ret)
+		return ret;
+
+	rcc = devm_kzalloc(&pdev->dev, sizeof(*rcc) + sizeof(*clks) * num_clks,
+			   GFP_KERNEL);
+	if (!rcc)
+		return -ENOMEM;
+
+	clks = rcc->clks;
+	data = &rcc->data;
+	data->clks = clks;
+	data->clk_num = num_clks;
+
+	clk = clk_register_fixed_rate(&pdev->dev, "sleep_clk_src", NULL,
+				      CLK_IS_ROOT, 32768);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	for (i = 0; i < num_clks; i++) {
+		if (!rpmcc_msm8916_clks[i]) {
+			clks[i] = ERR_PTR(-ENOENT);
+			continue;
+		}
+
+		rpmcc_msm8916_clks[i]->rpm = rpm;
+		clk = devm_clk_register(&pdev->dev, &rpmcc_msm8916_clks[i]->hw);
+		if (IS_ERR(clk))
+			return PTR_ERR(clk);
+
+		clks[i] = clk;
+	}
+
+	ret = of_clk_add_provider(pdev->dev.of_node, of_clk_src_onecell_get,
+				  data);
+	if (ret)
+		return ret;
+
+	/* Hold a vote for max rates */
+	clk_set_rate(bimc_a_clk.hw.clk, INT_MAX);
+	clk_prepare_enable(bimc_a_clk.hw.clk);
+	clk_set_rate(bimc_clk.hw.clk, INT_MAX);
+	clk_prepare_enable(bimc_clk.hw.clk);
+	clk_set_rate(snoc_clk.hw.clk, INT_MAX);
+	clk_prepare_enable(snoc_clk.hw.clk);
+	clk_prepare_enable(xo.hw.clk);
+
+	return 0;
+}
+
+static int rpmcc_msm8916_remove(struct platform_device *pdev)
+{
+	of_clk_del_provider(pdev->dev.of_node);
+	return 0;
+}
+
+static const struct of_device_id rpmcc_msm8916_match_table[] = {
+	{ .compatible = "qcom,rpmcc-msm8916" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, rpmcc_msm8916_match_table);
+
+static struct platform_driver rpmcc_msm8916_driver = {
+	.driver = {
+		.name = "qcom-rpmcc-msm8916",
+		.of_match_table = rpmcc_msm8916_match_table,
+	},
+	.probe = rpmcc_msm8916_probe,
+	.remove = rpmcc_msm8916_remove,
+};
+
+static int __init rpmcc_msm8916_init(void)
+{
+	return platform_driver_register(&rpmcc_msm8916_driver);
+}
+core_initcall(rpmcc_msm8916_init);
+
+static void __exit rpmcc_msm8916_exit(void)
+{
+	platform_driver_unregister(&rpmcc_msm8916_driver);
+}
+module_exit(rpmcc_msm8916_exit);
+
+MODULE_DESCRIPTION("Qualcomm MSM8916 RPM Clock Controller Driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:rpmcc-msm8916");
diff --git a/include/dt-bindings/clock/qcom,rpmcc-msm8916.h b/include/dt-bindings/clock/qcom,rpmcc-msm8916.h
new file mode 100644
index 000000000000..62d63940896a
--- /dev/null
+++ b/include/dt-bindings/clock/qcom,rpmcc-msm8916.h
@@ -0,0 +1,44 @@
+/*
+ * Copyright 2015 Linaro Limited
+ *
+ * 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 _DT_BINDINGS_CLK_MSM_RPMCC_8916_H
+#define _DT_BINDINGS_CLK_MSM_RPMCC_8916_H
+
+#define RPM_XO_CLK_SRC				0
+#define RPM_XO_A_CLK_SRC			1
+#define RPM_PCNOC_CLK				2
+#define RPM_PCNOC_A_CLK				3
+#define RPM_SNOC_CLK				4
+#define RPM_SNOC_A_CLK				6
+#define RPM_BIMC_CLK				7
+#define RPM_BIMC_A_CLK				8
+#define RPM_QDSS_CLK				9
+#define RPM_QDSS_A_CLK				10
+#define RPM_BB_CLK1				11
+#define RPM_BB_CLK1_A				12
+#define RPM_BB_CLK2				13
+#define RPM_BB_CLK2_A				14
+#define RPM_RF_CLK1				15
+#define RPM_RF_CLK1_A				16
+#define RPM_RF_CLK2				17
+#define RPM_RF_CLK2_A				18
+#define RPM_BB_CLK1_PIN				19
+#define RPM_BB_CLK1_A_PIN			20
+#define RPM_BB_CLK2_PIN				21
+#define RPM_BB_CLK2_A_PIN			22
+#define RPM_RF_CLK1_PIN				23
+#define RPM_RF_CLK1_A_PIN			24
+#define RPM_RF_CLK2_PIN				25
+#define RPM_RF_CLK2_A_PIN			26
+
+#endif

Re: [PATCH v2 2/2] clk: qcom: Add MSM8916 RPM clock driver

[Stephen Boyd]

On 08/03, Georgi Djakov wrote:
> diff --git a/Documentation/devicetree/bindings/clock/qcom,rpmcc.txt b/Documentation/devicetree/bindings/clock/qcom,rpmcc.txt
> new file mode 100644
> index 000000000000..bd0fd0cd50dc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/qcom,rpmcc.txt

Binding looks good.

> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> index e347b97aa9c7..edffb57a3aff 100644
> --- a/drivers/clk/qcom/Kconfig
> +++ b/drivers/clk/qcom/Kconfig
> @@ -52,6 +52,7 @@ config MSM_GCC_8660
>  
>  config MSM_GCC_8916
>  	tristate "MSM8916 Global Clock Controller"
> +	select QCOM_CLK_SMD_RPM

This will probably introduce some sort of build failure if the
RPM SMD driver is not compiled in?

>  	depends on COMMON_CLK_QCOM
>  	help
>  	  Support for the global clock controller on msm8916 devices.
> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
> index 33adf1d97da3..985c794608a7 100644
> --- a/drivers/clk/qcom/Makefile
> +++ b/drivers/clk/qcom/Makefile
> @@ -17,6 +17,7 @@ obj-$(CONFIG_IPQ_GCC_806X) += gcc-ipq806x.o
>  obj-$(CONFIG_IPQ_LCC_806X) += lcc-ipq806x.o
>  obj-$(CONFIG_MSM_GCC_8660) += gcc-msm8660.o
>  obj-$(CONFIG_MSM_GCC_8916) += gcc-msm8916.o
> +obj-$(CONFIG_MSM_GCC_8916) += rpmcc-msm8916.o

Bad config. Also, perhaps we can try to make it generic across
all SMD RPM based platforms? I don't think much changes between
chips to warrant a new driver every time for the RPMCC.

>  obj-$(CONFIG_MSM_GCC_8960) += gcc-msm8960.o
>  obj-$(CONFIG_MSM_LCC_8960) += lcc-msm8960.o
>  obj-$(CONFIG_MSM_GCC_8974) += gcc-msm8974.o
> diff --git a/drivers/clk/qcom/gcc-msm8916.c b/drivers/clk/qcom/gcc-msm8916.c
> index 3bf4fb3deef6..28ef2c771157 100644
> --- a/drivers/clk/qcom/gcc-msm8916.c
> +++ b/drivers/clk/qcom/gcc-msm8916.c
> @@ -2820,19 +2820,6 @@ MODULE_DEVICE_TABLE(of, gcc_msm8916_match_table);
>  
>  static int gcc_msm8916_probe(struct platform_device *pdev)
>  {
> -	struct clk *clk;
> -	struct device *dev = &pdev->dev;
> -
> -	/* Temporary until RPM clocks supported */
> -	clk = clk_register_fixed_rate(dev, "xo", NULL, CLK_IS_ROOT, 19200000);
> -	if (IS_ERR(clk))
> -		return PTR_ERR(clk);

Hmm.. this problem. With this change we're going to make the gcc
driver depend on the RPM driver (and if we continue along this
path we should make the Kconfig reflect that).

I wonder if we could do something like this instead:

 1) Introduce a fixed rate 'xo_board' clock into DT that has the rate of
 the XO resource.

 2) If the RPM clock driver isn't enabled, register a dummy pass
 through clock (fixed factor of 1 perhaps?) called 'xo' in the gcc
 driver.

 3) If the RPM clock driver is enabled, we won't register the 
 clock above and we'll make sure to set the parent of the xo
 clock in the RPM clock driver to xo_board.

The benefit of this complicated scheme is that the rate of the XO
clock is not hard-coded in the RPM driver or gcc driver (it comes
from the board layout anyway so it should be in DT), and we also
work seamlessly in the case where the RPM clock driver isn't
enabled. This makes for a smooth transition.

If we were to apply this patch right now, 8916 would stop booting
until the RPM clock driver was enabled. What a mess!

> -
> -	clk = clk_register_fixed_rate(dev, "sleep_clk_src", NULL,
> -				      CLK_IS_ROOT, 32768);
> -	if (IS_ERR(clk))
> -		return PTR_ERR(clk);
> -
>  	return qcom_cc_probe(pdev, &gcc_msm8916_desc);
>  }
>  
> diff --git a/drivers/clk/qcom/rpmcc-msm8916.c b/drivers/clk/qcom/rpmcc-msm8916.c
> new file mode 100644
> index 000000000000..60b2292dcd45
> --- /dev/null
> +++ b/drivers/clk/qcom/rpmcc-msm8916.c
> @@ -0,0 +1,196 @@
> +
> +static int rpmcc_msm8916_probe(struct platform_device *pdev)
> +{
> +	struct clk **clks;
> +	struct clk *clk;
> +	struct rpm_cc *rcc;
> +	struct qcom_smd_rpm *rpm;
> +	struct clk_onecell_data *data;
> +	int num_clks = ARRAY_SIZE(rpmcc_msm8916_clks);
> +	int ret, i;
> +
> +	if (!pdev->dev.of_node)
> +		return -ENODEV;

This never happens. Please remove.

> +
> +	rpm = dev_get_drvdata(pdev->dev.parent);
> +	if (!rpm) {
> +		dev_err(&pdev->dev, "Unable to retrieve handle to RPM\n");
> +		return -ENODEV;
> +	}
> +
> +	ret = clk_smd_rpm_enable_scaling(rpm);
> +	if (ret)
> +		return ret;
> +
> +	rcc = devm_kzalloc(&pdev->dev, sizeof(*rcc) + sizeof(*clks) * num_clks,
> +			   GFP_KERNEL);
> +	if (!rcc)
> +		return -ENOMEM;

Maybe we should do this before we enable RPM scaling.

> +
> +	clks = rcc->clks;
> +	data = &rcc->data;
> +	data->clks = clks;
> +	data->clk_num = num_clks;
> +
> +	clk = clk_register_fixed_rate(&pdev->dev, "sleep_clk_src", NULL,
> +				      CLK_IS_ROOT, 32768);
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);

We should move the sleep_clk_src to DT. No need to register it
here. Please make that a separate patch. I imagine we'll need to
make it so that registering the sleep_clk_src doesn't fail the
gcc probe, add the DT clock entry, and then remove the fixed rate
clock registration from the gcc probe after that.

> +
> +	for (i = 0; i < num_clks; i++) {
> +		if (!rpmcc_msm8916_clks[i]) {
> +			clks[i] = ERR_PTR(-ENOENT);
> +			continue;
> +		}
> +
> +		rpmcc_msm8916_clks[i]->rpm = rpm;
> +		clk = devm_clk_register(&pdev->dev, &rpmcc_msm8916_clks[i]->hw);
> +		if (IS_ERR(clk))
> +			return PTR_ERR(clk);
> +
> +		clks[i] = clk;
> +	}
> +
> +	ret = of_clk_add_provider(pdev->dev.of_node, of_clk_src_onecell_get,
> +				  data);
> +	if (ret)
> +		return ret;
> +
> +	/* Hold a vote for max rates */
> +	clk_set_rate(bimc_a_clk.hw.clk, INT_MAX);
> +	clk_prepare_enable(bimc_a_clk.hw.clk);
> +	clk_set_rate(bimc_clk.hw.clk, INT_MAX);
> +	clk_prepare_enable(bimc_clk.hw.clk);
> +	clk_set_rate(snoc_clk.hw.clk, INT_MAX);
> +	clk_prepare_enable(snoc_clk.hw.clk);
> +	clk_prepare_enable(xo.hw.clk);

This is a combination of critical clocks and assigned-rates.
Critical clocks aren't here yet, hopefully soon. Assigned-rates
are here though, so can we use those to set these clocks to some
max speed?

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

Re: [PATCH v2 0/2] Add initial support for RPM clocks

[Georgi Djakov]

On 08/03/2015 07:48 PM, Georgi Djakov wrote:
> This patchset adds initial support for the clocks controlled by
> the RPM (Resource Power Manager) processor on Qualcomm platforms.
> It depends on Bjorn's Qualcomm SMD & RPM patches, that are now in
> linux-next: https://lkml.org/lkml/2015/7/27/1125
> 
> The first patch implements the RPM clock operations, which are
> using a shared memory for sending requests to the RPM processor.
> The second patch adds the support of RPM clocks on the MSM8916
> platform.
> 
> Changes since v1 (https://lkml.org/lkml/2015/7/9/257):
> * Changed the driver name to clk-smd-rpm, also build it only
>   when it is needed - suggested by Srini and Bjorn.
> * More detailed binding example.
> * Minor changes.
> 
> Georgi Djakov (2):
>   clk: qcom: Add support for RPM Clocks
>   clk: qcom: Add MSM8916 RPM clock driver
> 
>  .../devicetree/bindings/clock/qcom,rpmcc.txt       |   35 ++++
>  drivers/clk/qcom/Kconfig                           |    4 +
>  drivers/clk/qcom/Makefile                          |    2 +
>  drivers/clk/qcom/clk-smd-rpm.c                     |  165 ++++++++++++++++
>  drivers/clk/qcom/clk-smd-rpm.h                     |  139 ++++++++++++++
>  drivers/clk/qcom/gcc-msm8916.c                     |   13 --
>  drivers/clk/qcom/rpmcc-msm8916.c                   |  196 ++++++++++++++++++++
>  include/dt-bindings/clock/qcom,rpmcc-msm8916.h     |   44 +++++
>  8 files changed, 585 insertions(+), 13 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/clock/qcom,rpmcc.txt
>  create mode 100644 drivers/clk/qcom/clk-smd-rpm.c
>  create mode 100644 drivers/clk/qcom/clk-smd-rpm.h
>  create mode 100644 drivers/clk/qcom/rpmcc-msm8916.c
>  create mode 100644 include/dt-bindings/clock/qcom,rpmcc-msm8916.h
> 

Hi Stephen, any comments on this?
Thanks!

Re: [PATCH v2 0/2] Add initial support for RPM clocks

[Bjorn Andersson]

On Mon 03 Aug 09:48 PDT 2015, Georgi Djakov wrote:

> This patchset adds initial support for the clocks controlled by
> the RPM (Resource Power Manager) processor on Qualcomm platforms.
> It depends on Bjorn's Qualcomm SMD & RPM patches, that are now in
> linux-next: https://lkml.org/lkml/2015/7/27/1125
> 
> The first patch implements the RPM clock operations, which are
> using a shared memory for sending requests to the RPM processor.
> The second patch adds the support of RPM clocks on the MSM8916
> platform.
> 

I pulled this set into my local tree and added a MSM8974 driver; so I
could enable cxo_a2 and load the WCNSS firmware. This works great.

I do however have a problem that if I revise the gcc and mmcc drivers to
be rooted in cxo_clk_src (like you have for 8916) the clk_set_rate() in
msm_serial messes up my uart - way before the rpmcc driver is loaded.
I have not debugged this further (as it wasn't needed for my usecase),
but have you seen any issues related to this?


Will you be able to send out v3 anytime soon, so I can rebase my 8974
patch and send that out? Or would you be interested in me sending you my
8974 and you can include that in your series later?

Regards,
Bjorn

Re: [PATCH v2 0/2] Add initial support for RPM clocks

[Bjorn Andersson]

On Mon 03 Aug 09:48 PDT 2015, Georgi Djakov wrote:

> This patchset adds initial support for the clocks controlled by
> the RPM (Resource Power Manager) processor on Qualcomm platforms.
> It depends on Bjorn's Qualcomm SMD & RPM patches, that are now in
> linux-next: https://lkml.org/lkml/2015/7/27/1125
> 
> The first patch implements the RPM clock operations, which are
> using a shared memory for sending requests to the RPM processor.
> The second patch adds the support of RPM clocks on the MSM8916
> platform.
> 

I pulled this set into my local tree and added a MSM8974 driver; so I
could enable cxo_a2 and load the WCNSS firmware. This works great.

I do however have a problem that if I revise the gcc and mmcc drivers to
be rooted in cxo_clk_src (like you have for 8916) the clk_set_rate() in
msm_serial messes up my uart - way before the rpmcc driver is loaded.
I have not debugged this further (as it wasn't needed for my usecase),
but have you seen any issues related to this?


Will you be able to send out v3 anytime soon, so I can rebase my 8974
patch and send that out? Or would you be interested in me sending you my
8974 and you can include that in your series later?

Regards,
Bjorn

Re: [PATCH v2 0/2] Add initial support for RPM clocks

[Georgi Djakov]

On 09/21/2015 07:33 AM, Bjorn Andersson wrote:
> On Mon 03 Aug 09:48 PDT 2015, Georgi Djakov wrote:
>
>> This patchset adds initial support for the clocks controlled by
>> the RPM (Resource Power Manager) processor on Qualcomm platforms.
>> It depends on Bjorn's Qualcomm SMD & RPM patches, that are now in
>> linux-next: https://lkml.org/lkml/2015/7/27/1125
>>
>> The first patch implements the RPM clock operations, which are
>> using a shared memory for sending requests to the RPM processor.
>> The second patch adds the support of RPM clocks on the MSM8916
>> platform.
>>
>
> I pulled this set into my local tree and added a MSM8974 driver; so I
> could enable cxo_a2 and load the WCNSS firmware. This works great.
>

Thanks for trying it.

> I do however have a problem that if I revise the gcc and mmcc drivers to
> be rooted in cxo_clk_src (like you have for 8916) the clk_set_rate() in
> msm_serial messes up my uart - way before the rpmcc driver is loaded.
> I have not debugged this further (as it wasn't needed for my usecase),
> but have you seen any issues related to this?
>

I am not seeing this on 8916 and don't remember seeing in on 8974, but 
will give it a try again next week when i will have a 8974 board.

>
> Will you be able to send out v3 anytime soon, so I can rebase my 8974
> patch and send that out? Or would you be interested in me sending you my
> 8974 and you can include that in your series later?

Yes, will send v3 probably next week. I also have a work in progress
patch for 8974, but don't mind taking yours instead.

BR,
Georgi