[PATCH 0/7] Add powerdomain driver for corners on msm8996

[PATCH 0/7] Add powerdomain driver for corners on msm8996

[Rajendra Nayak]

With performance state support for genpd merged, and with some more
patches to add support for them in the OPP layer [1] under discussion,
this is an effort to model a powerdomain driver to communicate corner/level
values for qualcomm platforms to RPM (Remote Power Manager)

This series adds data specific to msm8996 and is tested on the
db820c. We also modify mmc as one of the first devices to move to
using an OPP table and vote on corners using the performance state
infrastructure.

[1] https://lwn.net/Articles/742136/

Rajendra Nayak (6):
  soc: qcom: rpmpd: Add a powerdomain driver to model corners
  dt-bindings: opp: Introduce qcom-opp bindings
  soc: qcom: rpmpd: Add support for get/set performance state
  arm64: dts: msm8996: Add rpmpd device node
  mmc: sdhci-msm: Adapt the driver to use OPPs to set clocks/performance
    state
  soc: qcom: rpmpd: Add a max vote on all corners at init

Viresh Kumar (1):
  PM / OPP: Add dev_pm_opp_get_of_node()

 Documentation/devicetree/bindings/opp/qcom-opp.txt |  25 ++
 .../devicetree/bindings/power/qcom,rpmpd.txt       |  14 +
 arch/arm64/boot/dts/qcom/msm8996.dtsi              |  87 +++++
 drivers/clk/qcom/gcc-msm8996.c                     |   8 +-
 drivers/mmc/host/sdhci-msm.c                       |  57 +++-
 drivers/opp/of.c                                   |  19 ++
 drivers/soc/qcom/Kconfig                           |   9 +
 drivers/soc/qcom/Makefile                          |   1 +
 drivers/soc/qcom/rpmpd.c                           | 350 +++++++++++++++++++++
 include/linux/pm_opp.h                             |   5 +
 10 files changed, 561 insertions(+), 14 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/opp/qcom-opp.txt
 create mode 100644 Documentation/devicetree/bindings/power/qcom,rpmpd.txt
 create mode 100644 drivers/soc/qcom/rpmpd.c

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 1/7] PM / OPP: Add dev_pm_opp_get_of_node()

[Rajendra Nayak]

From: Viresh Kumar <viresh.kumar@linaro.org>

This adds a new helper to let the power domain drivers to access
opp->np, so that they can read platform specific properties from the
node.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 drivers/opp/of.c       | 19 +++++++++++++++++++
 include/linux/pm_opp.h |  5 +++++
 2 files changed, 24 insertions(+)

diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index 21265af55117..b17715bc3c0a 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -736,3 +736,22 @@ struct dev_pm_opp *of_dev_pm_opp_find_required_opp(struct device *dev,
 	return opp;
 }
 EXPORT_SYMBOL_GPL(of_dev_pm_opp_find_required_opp);
+
+/**
+ * dev_pm_opp_get_of_node() - Gets the DT node corresponding to an opp
+ * @opp:	opp for which DT node has to be returned for
+ *
+ * Return: DT node corresponding to the opp, else 0 on success.
+ *
+ * The caller needs to put the node of_node_put() after using it.
+ */
+struct device_node *dev_pm_opp_get_of_node(struct dev_pm_opp *opp)
+{
+	if (IS_ERR_OR_NULL(opp)) {
+		pr_err("%s: Invalid parameters\n", __func__);
+		return NULL;
+	}
+
+	return of_node_get(opp->np);
+}
+EXPORT_SYMBOL_GPL(dev_pm_opp_get_of_node);
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 528a7d9cf2ef..099b31960dec 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -300,6 +300,7 @@ void dev_pm_opp_of_cpumask_remove_table(const struct cpumask *cpumask);
 int dev_pm_opp_of_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask);
 struct device_node *dev_pm_opp_of_get_opp_desc_node(struct device *dev);
 struct dev_pm_opp *of_dev_pm_opp_find_required_opp(struct device *dev, struct device_node *np);
+struct device_node *dev_pm_opp_get_of_node(struct dev_pm_opp *opp);
 #else
 static inline int dev_pm_opp_of_add_table(struct device *dev)
 {
@@ -338,6 +339,10 @@ static inline struct dev_pm_opp *of_dev_pm_opp_find_required_opp(struct device *
 {
 	return NULL;
 }
+static inline struct device_node *dev_pm_opp_get_of_node(struct dev_pm_opp *opp)
+{
+	return NULL;
+}
 #endif
 
 #endif		/* __LINUX_OPP_H__ */
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 1/7] PM / OPP: Add dev_pm_opp_get_of_node()

[Stephen Boyd]

Quoting Rajendra Nayak (2018-03-15 21:08:18)
> From: Viresh Kumar <viresh.kumar@linaro.org>
> 
> This adds a new helper to let the power domain drivers to access
> opp->np, so that they can read platform specific properties from the
> node.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---

Are there two versions of this patch floating around?

>  drivers/opp/of.c       | 19 +++++++++++++++++++
>  include/linux/pm_opp.h |  5 +++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/drivers/opp/of.c b/drivers/opp/of.c
> index 21265af55117..b17715bc3c0a 100644
> --- a/drivers/opp/of.c
> +++ b/drivers/opp/of.c
> @@ -736,3 +736,22 @@ struct dev_pm_opp *of_dev_pm_opp_find_required_opp(struct device *dev,
>         return opp;
>  }
>  EXPORT_SYMBOL_GPL(of_dev_pm_opp_find_required_opp);
> +
> +/**
> + * dev_pm_opp_get_of_node() - Gets the DT node corresponding to an opp
> + * @opp:       opp for which DT node has to be returned for
> + *
> + * Return: DT node corresponding to the opp, else 0 on success.
> + *
> + * The caller needs to put the node of_node_put() after using it.

put the node with of_node_put()

Re: [PATCH 1/7] PM / OPP: Add dev_pm_opp_get_of_node()

[Viresh Kumar]

On 07-04-18, 06:00, Stephen Boyd wrote:
> Quoting Rajendra Nayak (2018-03-15 21:08:18)
> > From: Viresh Kumar <viresh.kumar@linaro.org>
> > 
> > This adds a new helper to let the power domain drivers to access
> > opp->np, so that they can read platform specific properties from the
> > node.
> > 
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> > ---
> 
> Are there two versions of this patch floating around?

Yeah, I will carry this patch in my series now going forward.

> >  drivers/opp/of.c       | 19 +++++++++++++++++++
> >  include/linux/pm_opp.h |  5 +++++
> >  2 files changed, 24 insertions(+)
> > 
> > diff --git a/drivers/opp/of.c b/drivers/opp/of.c
> > index 21265af55117..b17715bc3c0a 100644
> > --- a/drivers/opp/of.c
> > +++ b/drivers/opp/of.c
> > @@ -736,3 +736,22 @@ struct dev_pm_opp *of_dev_pm_opp_find_required_opp(struct device *dev,
> >         return opp;
> >  }
> >  EXPORT_SYMBOL_GPL(of_dev_pm_opp_find_required_opp);
> > +
> > +/**
> > + * dev_pm_opp_get_of_node() - Gets the DT node corresponding to an opp
> > + * @opp:       opp for which DT node has to be returned for
> > + *
> > + * Return: DT node corresponding to the opp, else 0 on success.
> > + *
> > + * The caller needs to put the node of_node_put() after using it.
> 
> put the node with of_node_put()

Fixed it up, thanks.

-- 
viresh

Re: [PATCH 1/7] PM / OPP: Add dev_pm_opp_get_of_node()

[Jordan Crouse]

On Sat, Apr 07, 2018 at 06:00:29AM -0700, Stephen Boyd wrote:
> Quoting Rajendra Nayak (2018-03-15 21:08:18)
> > From: Viresh Kumar <viresh.kumar@linaro.org>
> > 
> > This adds a new helper to let the power domain drivers to access
> > opp->np, so that they can read platform specific properties from the
> > node.
> > 
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> > ---
> 
> Are there two versions of this patch floating around?

The other one was mine, but it is identical to this.

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

[PATCH 2/7] soc: qcom: rpmpd: Add a powerdomain driver to model corners

[Rajendra Nayak]

The powerdomains for corners just pass the performance state set by the
consumers to the RPM (Remote Power manager) which then takes care
of setting the appropriate voltage on the corresponding rails to
meet the performance needs.

We add all powerdomain data needed on msm8996 here. This driver can easily
be extended by adding data for other qualcomm SoCs as well.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 .../devicetree/bindings/power/qcom,rpmpd.txt       |  14 +
 drivers/soc/qcom/Kconfig                           |   9 +
 drivers/soc/qcom/Makefile                          |   1 +
 drivers/soc/qcom/rpmpd.c                           | 299 +++++++++++++++++++++
 4 files changed, 323 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/qcom,rpmpd.txt
 create mode 100644 drivers/soc/qcom/rpmpd.c

diff --git a/Documentation/devicetree/bindings/power/qcom,rpmpd.txt b/Documentation/devicetree/bindings/power/qcom,rpmpd.txt
new file mode 100644
index 000000000000..4d7a4a1f054f
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/qcom,rpmpd.txt
@@ -0,0 +1,14 @@
+Qualcomm RPM Powerdomains
+
+* For RPM powerdomains, we communicate a performance state to RPM
+which then translates it into a corresponding voltage on a rail
+
+Required Properties:
+ - compatible: Should be one of the following
+	* qcom,rpmpd-msm8996: RPM Powerdomain for the msm8996 family of SoC
+ - power-domain-cells: number of cells in power domain specifier
+	must be 1.
+ - domain-performance-state: One or more nodes describing the performance-states
+	supported by the powerdomain.
+	For domain-performance-state bindings refer to
+	Documentation/devicetree/bindings/power/power_domain.txt
diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index e050eb83341d..6e1a0ed7ea3f 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -55,6 +55,15 @@ config QCOM_RMTFS_MEM
 
 	  Say y here if you intend to boot the modem remoteproc.
 
+config QCOM_RPMPD
+	tristate "Qualcomm RPM Powerdomain driver"
+	depends on MFD_QCOM_RPM && QCOM_SMD_RPM
+	help
+	  QCOM RPM powerdomain driver to support powerdomain with
+	  performance states. The driver communicates a performance state
+	  value to RPM which then translates it into corresponding voltage
+	  for the voltage rail.
+
 config QCOM_SMEM
 	tristate "Qualcomm Shared Memory Manager (SMEM)"
 	depends on ARCH_QCOM
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index dcebf2814e6d..12c48a3ea106 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -12,3 +12,4 @@ obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o
 obj-$(CONFIG_QCOM_SMP2P)	+= smp2p.o
 obj-$(CONFIG_QCOM_SMSM)	+= smsm.o
 obj-$(CONFIG_QCOM_WCNSS_CTRL) += wcnss_ctrl.o
+obj-$(CONFIG_QCOM_RPMPD) += rpmpd.o
diff --git a/drivers/soc/qcom/rpmpd.c b/drivers/soc/qcom/rpmpd.c
new file mode 100644
index 000000000000..c8754d867c33
--- /dev/null
+++ b/drivers/soc/qcom/rpmpd.c
@@ -0,0 +1,299 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/err.h>
+#include <linux/export.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/pm_domain.h>
+#include <linux/mfd/qcom_rpm.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/soc/qcom/smd-rpm.h>
+
+#include <dt-bindings/mfd/qcom-rpm.h>
+
+#define domain_to_rpmpd(domain) container_of(domain, struct rpmpd, pd)
+
+/* Resource types */
+#define RPMPD_SMPA 0x61706d73
+#define RPMPD_LDOA 0x616f646c
+
+/* Operation Keys */
+#define KEY_CORNER		0x6e726f63 /* corn */
+#define KEY_ENABLE		0x6e657773 /* swen */
+#define KEY_FLOOR_CORNER	0x636676   /* vfc */
+
+#define DEFINE_RPMPD_CORN_SMPA(_platform, _name, _active, r_id)		\
+	static struct rpmpd _platform##_##_active;			\
+	static struct rpmpd _platform##_##_name = {			\
+		.pd = {	.name = #_name,	},				\
+		.peer = &_platform##_##_active,				\
+		.res_type = RPMPD_SMPA,					\
+		.res_id = r_id,						\
+		.key = KEY_CORNER,					\
+	};								\
+	static struct rpmpd _platform##_##_active = {			\
+		.pd = { .name = #_active, },				\
+		.peer = &_platform##_##_name,				\
+		.active_only = true,					\
+		.res_type = RPMPD_SMPA,					\
+		.res_id = r_id,						\
+		.key = KEY_CORNER,					\
+	}
+
+#define DEFINE_RPMPD_CORN_LDOA(_platform, _name, r_id)			\
+	static struct rpmpd _platform##_##_name = {			\
+		.pd = { .name = #_name, },				\
+		.res_type = RPMPD_LDOA,					\
+		.res_id = r_id,						\
+		.key = KEY_CORNER,					\
+	}
+
+#define DEFINE_RPMPD_VFC(_platform, _name, r_id, r_type)		\
+	static struct rpmpd _platform##_##_name = {			\
+		.pd = { .name = #_name, },				\
+		.res_type = r_type,					\
+		.res_id = r_id,						\
+		.key = KEY_FLOOR_CORNER,				\
+	}
+
+#define DEFINE_RPMPD_VFC_SMPA(_platform, _name, r_id)			\
+	DEFINE_RPMPD_VFC(_platform, _name, r_id, RPMPD_SMPA)
+
+#define DEFINE_RPMPD_VFC_LDOA(_platform, _name, r_id)			\
+	DEFINE_RPMPD_VFC(_platform, _name, r_id, RPMPD_LDOA)
+
+struct rpmpd_req {
+	__le32 key;
+	__le32 nbytes;
+	__le32 value;
+};
+
+struct rpmpd {
+	struct generic_pm_domain pd;
+	struct rpmpd *peer;
+	const bool active_only;
+	unsigned long corner;
+	bool enabled;
+	const char *res_name;
+	const int res_type;
+	const int res_id;
+	struct qcom_smd_rpm *rpm;
+	__le32 key;
+};
+
+struct rpmpd_desc {
+	struct rpmpd **rpmpds;
+	size_t num_pds;
+};
+
+static DEFINE_MUTEX(rpmpd_lock);
+
+/* msm8996 RPM powerdomains */
+DEFINE_RPMPD_CORN_SMPA(msm8996, vddcx, vddcx_ao, 1);
+DEFINE_RPMPD_CORN_SMPA(msm8996, vddmx, vddmx_ao, 2);
+DEFINE_RPMPD_CORN_LDOA(msm8996, vddsscx, 26);
+
+DEFINE_RPMPD_VFC_SMPA(msm8996, vddcx_vfc, 1);
+DEFINE_RPMPD_VFC_LDOA(msm8996, vddsscx_vfc, 26);
+
+static struct rpmpd *msm8996_rpmpds[] = {
+	[0] = &msm8996_vddcx,
+	[1] = &msm8996_vddcx_ao,
+	[2] = &msm8996_vddcx_vfc,
+	[3] = &msm8996_vddmx,
+	[4] = &msm8996_vddmx_ao,
+	[5] = &msm8996_vddsscx,
+	[6] = &msm8996_vddsscx_vfc,
+};
+
+static const struct rpmpd_desc msm8996_desc = {
+	.rpmpds = msm8996_rpmpds,
+	.num_pds = ARRAY_SIZE(msm8996_rpmpds),
+};
+
+static const struct of_device_id rpmpd_match_table[] = {
+	{ .compatible = "qcom,rpmpd-msm8996", .data = &msm8996_desc },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, rpmpd_match_table);
+
+static int rpmpd_send_enable(struct rpmpd *pd, bool enable)
+{
+	struct rpmpd_req req = {
+		.key = KEY_ENABLE,
+		.nbytes = cpu_to_le32(sizeof(u32)),
+		.value = cpu_to_le32(enable),
+	};
+
+	return qcom_rpm_smd_write(pd->rpm, QCOM_RPM_ACTIVE_STATE, pd->res_type,
+				  pd->res_id, &req, sizeof(req));
+}
+
+static int rpmpd_send_corner(struct rpmpd *pd, int state, unsigned int corner)
+{
+	struct rpmpd_req req = {
+		.key = pd->key,
+		.nbytes = cpu_to_le32(sizeof(u32)),
+		.value = cpu_to_le32(corner),
+	};
+
+	return qcom_rpm_smd_write(pd->rpm, state, pd->res_type, pd->res_id,
+				  &req, sizeof(req));
+};
+
+static void to_active_sleep(struct rpmpd *pd, unsigned long corner,
+			    unsigned long *active, unsigned long *sleep)
+{
+	*active = corner;
+
+	if (pd->active_only)
+		*sleep = 0;
+	else
+		*sleep = *active;
+}
+
+static int rpmpd_aggregate_corner(struct rpmpd *pd)
+{
+	int ret;
+	struct rpmpd *peer = pd->peer;
+	unsigned long active_corner, sleep_corner;
+	unsigned long this_corner = 0, this_sleep_corner = 0;
+	unsigned long peer_corner = 0, peer_sleep_corner = 0;
+
+	to_active_sleep(pd, pd->corner, &this_corner, &this_sleep_corner);
+
+	if (peer && peer->enabled)
+		to_active_sleep(peer, peer->corner, &peer_corner,
+				&peer_sleep_corner);
+
+	active_corner = max(this_corner, peer_corner);
+
+	ret = rpmpd_send_corner(pd, QCOM_RPM_ACTIVE_STATE, active_corner);
+	if (ret)
+		return ret;
+
+	sleep_corner = max(this_sleep_corner, peer_sleep_corner);
+
+	return rpmpd_send_corner(pd, QCOM_RPM_SLEEP_STATE, sleep_corner);
+}
+
+static int rpmpd_power_on(struct generic_pm_domain *domain)
+{
+	int ret;
+	struct rpmpd *pd = domain_to_rpmpd(domain);
+
+	mutex_lock(&rpmpd_lock);
+
+	ret = rpmpd_send_enable(pd, true);
+	if (ret)
+		goto out;
+
+	pd->enabled = true;
+
+	if (pd->corner)
+		ret = rpmpd_aggregate_corner(pd);
+
+out:
+	mutex_unlock(&rpmpd_lock);
+
+	return ret;
+}
+
+static int rpmpd_power_off(struct generic_pm_domain *domain)
+{
+	int ret;
+	struct rpmpd *pd = domain_to_rpmpd(domain);
+
+	mutex_lock(&rpmpd_lock);
+
+	ret = rpmpd_send_enable(pd, false);
+	if (!ret)
+		pd->enabled = false;
+
+	mutex_unlock(&rpmpd_lock);
+
+	return ret;
+}
+
+static int rpmpd_probe(struct platform_device *pdev)
+{
+	int i;
+	size_t num;
+	struct genpd_onecell_data *data;
+	struct qcom_smd_rpm *rpm;
+	struct rpmpd **rpmpds;
+	const struct rpmpd_desc *desc;
+
+	rpm = dev_get_drvdata(pdev->dev.parent);
+	if (!rpm) {
+		dev_err(&pdev->dev, "Unable to retrieve handle to RPM\n");
+		return -ENODEV;
+	}
+
+	desc = of_device_get_match_data(&pdev->dev);
+	if (!desc)
+		return -EINVAL;
+
+	rpmpds = desc->rpmpds;
+	num = desc->num_pds;
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->domains = devm_kcalloc(&pdev->dev, num, sizeof(*data->domains),
+				     GFP_KERNEL);
+	data->num_domains = num;
+
+	for (i = 0; i < num; i++) {
+		if (!rpmpds[i])
+			continue;
+
+		rpmpds[i]->rpm = rpm;
+		rpmpds[i]->pd.power_off = rpmpd_power_off;
+		rpmpds[i]->pd.power_on = rpmpd_power_on;
+		pm_genpd_init(&rpmpds[i]->pd, NULL, true);
+
+		data->domains[i] = &rpmpds[i]->pd;
+	}
+
+	return of_genpd_add_provider_onecell(pdev->dev.of_node, data);
+}
+
+static int rpmpd_remove(struct platform_device *pdev)
+{
+	of_genpd_del_provider(pdev->dev.of_node);
+	return 0;
+}
+
+static struct platform_driver rpmpd_driver = {
+	.driver = {
+		.name = "qcom-rpmpd",
+		.of_match_table = rpmpd_match_table,
+	},
+	.probe = rpmpd_probe,
+	.remove = rpmpd_remove,
+};
+
+static int __init rpmpd_init(void)
+{
+	return platform_driver_register(&rpmpd_driver);
+}
+core_initcall(rpmpd_init);
+
+static void __exit rpmpd_exit(void)
+{
+	platform_driver_unregister(&rpmpd_driver);
+}
+module_exit(rpmpd_exit);
+
+MODULE_DESCRIPTION("Qualcomm RPM Power Domain Driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:qcom-rpmpd");
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 2/7] soc: qcom: rpmpd: Add a powerdomain driver to model corners

[Rob Herring]

On Fri, Mar 16, 2018 at 09:38:19AM +0530, Rajendra Nayak wrote:
> The powerdomains for corners just pass the performance state set by the
> consumers to the RPM (Remote Power manager) which then takes care
> of setting the appropriate voltage on the corresponding rails to
> meet the performance needs.
> 
> We add all powerdomain data needed on msm8996 here. This driver can easily
> be extended by adding data for other qualcomm SoCs as well.
> 
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  .../devicetree/bindings/power/qcom,rpmpd.txt       |  14 +
>  drivers/soc/qcom/Kconfig                           |   9 +
>  drivers/soc/qcom/Makefile                          |   1 +
>  drivers/soc/qcom/rpmpd.c                           | 299 +++++++++++++++++++++
>  4 files changed, 323 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/qcom,rpmpd.txt
>  create mode 100644 drivers/soc/qcom/rpmpd.c
> 
> diff --git a/Documentation/devicetree/bindings/power/qcom,rpmpd.txt b/Documentation/devicetree/bindings/power/qcom,rpmpd.txt
> new file mode 100644
> index 000000000000..4d7a4a1f054f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/qcom,rpmpd.txt
> @@ -0,0 +1,14 @@
> +Qualcomm RPM Powerdomains
> +
> +* For RPM powerdomains, we communicate a performance state to RPM
> +which then translates it into a corresponding voltage on a rail
> +
> +Required Properties:
> + - compatible: Should be one of the following
> +	* qcom,rpmpd-msm8996: RPM Powerdomain for the msm8996 family of SoC

qcom,msm8996-rpmpd

> + - power-domain-cells: number of cells in power domain specifier
> +	must be 1.
> + - domain-performance-state: One or more nodes describing the performance-states
> +	supported by the powerdomain.
> +	For domain-performance-state bindings refer to
> +	Documentation/devicetree/bindings/power/power_domain.txt

[PATCH 3/7] dt-bindings: opp: Introduce qcom-opp bindings

[Rajendra Nayak]

On Qualcomm platforms, an OPP node needs to describe an
additional level/corner value that is then communicated to
a remote microprocessor by the CPU, which then takes some
actions (like adjusting voltage values across various rails)
based on the value passed.

Describe these bindings in the qcom-opp bindings document.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 Documentation/devicetree/bindings/opp/qcom-opp.txt | 25 ++++++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/opp/qcom-opp.txt

diff --git a/Documentation/devicetree/bindings/opp/qcom-opp.txt b/Documentation/devicetree/bindings/opp/qcom-opp.txt
new file mode 100644
index 000000000000..56fe87751881
--- /dev/null
+++ b/Documentation/devicetree/bindings/opp/qcom-opp.txt
@@ -0,0 +1,25 @@
+Qualcomm OPP bindings to descibe OPP nodes with corner/level values
+
+OPP tables for devices on qualcomm platforms require an additional
+platform specific corner/level value to be specified.
+This value is passed on to the RPM (Resource Power Manager) by
+the CPU, which then takes the necessary actions to set a voltage
+rail to a appropriate voltage based on the value passed.
+
+The bindings are based on top of the operating-points-v2 bindings
+described in Documentation/devicetree/bindings/opp/opp.txt
+Additional properties are described below.
+
+* OPP Table Node
+
+Required properties:
+- compatible: Allow OPPs to express their compatibility. It should be:
+  "operating-points-v2-qcom"
+
+* OPP Node
+
+Required properties:
+- qcom,level: On Qualcomm platforms an OPP node can describe a positive value
+representing a corner/level thats comminicated with a remote microprocessor
+(Usually called the RPM) which then translates it into a certain voltage on
+a voltage rail.
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 3/7] dt-bindings: opp: Introduce qcom-opp bindings

[Viresh Kumar]

On 16-03-18, 09:38, Rajendra Nayak wrote:
> On Qualcomm platforms, an OPP node needs to describe an
> additional level/corner value that is then communicated to
> a remote microprocessor by the CPU, which then takes some
> actions (like adjusting voltage values across various rails)
> based on the value passed.
> 
> Describe these bindings in the qcom-opp bindings document.
> 
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/opp/qcom-opp.txt | 25 ++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/opp/qcom-opp.txt
> 
> diff --git a/Documentation/devicetree/bindings/opp/qcom-opp.txt b/Documentation/devicetree/bindings/opp/qcom-opp.txt
> new file mode 100644
> index 000000000000..56fe87751881
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/opp/qcom-opp.txt
> @@ -0,0 +1,25 @@
> +Qualcomm OPP bindings to descibe OPP nodes with corner/level values
> +
> +OPP tables for devices on qualcomm platforms require an additional
> +platform specific corner/level value to be specified.
> +This value is passed on to the RPM (Resource Power Manager) by
> +the CPU, which then takes the necessary actions to set a voltage
> +rail to a appropriate voltage based on the value passed.
> +
> +The bindings are based on top of the operating-points-v2 bindings
> +described in Documentation/devicetree/bindings/opp/opp.txt
> +Additional properties are described below.
> +
> +* OPP Table Node
> +
> +Required properties:
> +- compatible: Allow OPPs to express their compatibility. It should be:
> +  "operating-points-v2-qcom"
> +
> +* OPP Node
> +
> +Required properties:
> +- qcom,level: On Qualcomm platforms an OPP node can describe a positive value
> +representing a corner/level thats comminicated with a remote microprocessor
> +(Usually called the RPM) which then translates it into a certain voltage on
> +a voltage rail.

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH 3/7] dt-bindings: opp: Introduce qcom-opp bindings

[Rob Herring]

On Fri, Mar 16, 2018 at 09:38:20AM +0530, Rajendra Nayak wrote:
> On Qualcomm platforms, an OPP node needs to describe an
> additional level/corner value that is then communicated to
> a remote microprocessor by the CPU, which then takes some
> actions (like adjusting voltage values across various rails)
> based on the value passed.
> 
> Describe these bindings in the qcom-opp bindings document.
> 
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/opp/qcom-opp.txt | 25 ++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/opp/qcom-opp.txt

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

Re: [PATCH 3/7] dt-bindings: opp: Introduce qcom-opp bindings

[Stephen Boyd]

Quoting Rajendra Nayak (2018-03-15 21:08:20)
> On Qualcomm platforms, an OPP node needs to describe an
> additional level/corner value that is then communicated to
> a remote microprocessor by the CPU, which then takes some
> actions (like adjusting voltage values across various rails)
> based on the value passed.
> 
> Describe these bindings in the qcom-opp bindings document.
> 
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---
> diff --git a/Documentation/devicetree/bindings/opp/qcom-opp.txt b/Documentation/devicetree/bindings/opp/qcom-opp.txt
> new file mode 100644
> index 000000000000..56fe87751881
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/opp/qcom-opp.txt
> @@ -0,0 +1,25 @@
> +Qualcomm OPP bindings to descibe OPP nodes with corner/level values
> +
> +OPP tables for devices on qualcomm platforms require an additional

Capitalize qualcomm?

> +platform specific corner/level value to be specified.
> +This value is passed on to the RPM (Resource Power Manager) by
> +the CPU, which then takes the necessary actions to set a voltage
> +rail to a appropriate voltage based on the value passed.

to an

> +
> +The bindings are based on top of the operating-points-v2 bindings
> +described in Documentation/devicetree/bindings/opp/opp.txt
> +Additional properties are described below.
> +
> +* OPP Table Node
> +
> +Required properties:
> +- compatible: Allow OPPs to express their compatibility. It should be:
> +  "operating-points-v2-qcom"

Can this be operating-points-v2-level or -qcom-level? Just worried that
qcom is too generic.

> +
> +* OPP Node
> +
> +Required properties:
> +- qcom,level: On Qualcomm platforms an OPP node can describe a positive value
> +representing a corner/level thats comminicated with a remote microprocessor

s/thats/that's/

> +(Usually called the RPM) which then translates it into a certain voltage on

lowercase Usually

> +a voltage rail.

[PATCH 4/7] soc: qcom: rpmpd: Add support for get/set performance state

[Rajendra Nayak]

With genpd now expecting powerdomain drivers supporting performance
state to support get/set performance state callbacks, add support for it
in the rpmpd driver.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/soc/qcom/rpmpd.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/drivers/soc/qcom/rpmpd.c b/drivers/soc/qcom/rpmpd.c
index c8754d867c33..4058c5b450c6 100644
--- a/drivers/soc/qcom/rpmpd.c
+++ b/drivers/soc/qcom/rpmpd.c
@@ -14,6 +14,7 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
+#include <linux/pm_opp.h>
 #include <linux/soc/qcom/smd-rpm.h>
 
 #include <dt-bindings/mfd/qcom-rpm.h>
@@ -29,6 +30,8 @@
 #define KEY_ENABLE		0x6e657773 /* swen */
 #define KEY_FLOOR_CORNER	0x636676   /* vfc */
 
+#define MAX_RPMPD_STATE		6
+
 #define DEFINE_RPMPD_CORN_SMPA(_platform, _name, _active, r_id)		\
 	static struct rpmpd _platform##_##_active;			\
 	static struct rpmpd _platform##_##_name = {			\
@@ -222,6 +225,43 @@ static int rpmpd_power_off(struct generic_pm_domain *domain)
 	return ret;
 }
 
+static int rpmpd_set_performance(struct generic_pm_domain *domain,
+			     unsigned int state)
+{
+	int ret = 0;
+	struct rpmpd *pd = domain_to_rpmpd(domain);
+
+	mutex_lock(&rpmpd_lock);
+
+	if (state > MAX_RPMPD_STATE)
+		goto out;
+
+	pd->corner = state;
+
+	if (!pd->enabled && (pd->key != KEY_FLOOR_CORNER))
+		goto out;
+
+	ret = rpmpd_aggregate_corner(pd);
+
+out:
+	mutex_unlock(&rpmpd_lock);
+
+	return ret;
+}
+
+static unsigned int rpmpd_get_performance(struct generic_pm_domain *genpd,
+					  struct dev_pm_opp *opp)
+{
+	struct device_node *np;
+	unsigned int corner = 0;
+
+	np = dev_pm_opp_get_of_node(opp);
+	of_property_read_u32(np, "qcom,level", &corner);
+	of_node_put(np);
+
+	return corner;
+}
+
 static int rpmpd_probe(struct platform_device *pdev)
 {
 	int i;
@@ -259,6 +299,8 @@ static int rpmpd_probe(struct platform_device *pdev)
 		rpmpds[i]->rpm = rpm;
 		rpmpds[i]->pd.power_off = rpmpd_power_off;
 		rpmpds[i]->pd.power_on = rpmpd_power_on;
+		rpmpds[i]->pd.set_performance_state = rpmpd_set_performance;
+		rpmpds[i]->pd.get_performance_state = rpmpd_get_performance;
 		pm_genpd_init(&rpmpds[i]->pd, NULL, true);
 
 		data->domains[i] = &rpmpds[i]->pd;
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 4/7] soc: qcom: rpmpd: Add support for get/set performance state

[Viresh Kumar]

On 16-03-18, 09:38, Rajendra Nayak wrote:
> With genpd now expecting powerdomain drivers supporting performance
> state to support get/set performance state callbacks, add support for it
> in the rpmpd driver.
> 
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/soc/qcom/rpmpd.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/drivers/soc/qcom/rpmpd.c b/drivers/soc/qcom/rpmpd.c
> index c8754d867c33..4058c5b450c6 100644
> --- a/drivers/soc/qcom/rpmpd.c
> +++ b/drivers/soc/qcom/rpmpd.c
> @@ -14,6 +14,7 @@
>  #include <linux/of.h>
>  #include <linux/of_device.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_opp.h>
>  #include <linux/soc/qcom/smd-rpm.h>
>  
>  #include <dt-bindings/mfd/qcom-rpm.h>
> @@ -29,6 +30,8 @@
>  #define KEY_ENABLE		0x6e657773 /* swen */
>  #define KEY_FLOOR_CORNER	0x636676   /* vfc */
>  
> +#define MAX_RPMPD_STATE		6
> +
>  #define DEFINE_RPMPD_CORN_SMPA(_platform, _name, _active, r_id)		\
>  	static struct rpmpd _platform##_##_active;			\
>  	static struct rpmpd _platform##_##_name = {			\
> @@ -222,6 +225,43 @@ static int rpmpd_power_off(struct generic_pm_domain *domain)
>  	return ret;
>  }
>  
> +static int rpmpd_set_performance(struct generic_pm_domain *domain,
> +			     unsigned int state)
> +{
> +	int ret = 0;
> +	struct rpmpd *pd = domain_to_rpmpd(domain);
> +
> +	mutex_lock(&rpmpd_lock);
> +
> +	if (state > MAX_RPMPD_STATE)
> +		goto out;
> +
> +	pd->corner = state;
> +
> +	if (!pd->enabled && (pd->key != KEY_FLOOR_CORNER))
> +		goto out;
> +
> +	ret = rpmpd_aggregate_corner(pd);
> +
> +out:
> +	mutex_unlock(&rpmpd_lock);
> +
> +	return ret;
> +}
> +
> +static unsigned int rpmpd_get_performance(struct generic_pm_domain *genpd,
> +					  struct dev_pm_opp *opp)
> +{
> +	struct device_node *np;
> +	unsigned int corner = 0;
> +
> +	np = dev_pm_opp_get_of_node(opp);
> +	of_property_read_u32(np, "qcom,level", &corner);

Don't we want to error out or do something else in case of an error ?

> +	of_node_put(np);
> +
> +	return corner;
> +}
> +
>  static int rpmpd_probe(struct platform_device *pdev)
>  {
>  	int i;
> @@ -259,6 +299,8 @@ static int rpmpd_probe(struct platform_device *pdev)
>  		rpmpds[i]->rpm = rpm;
>  		rpmpds[i]->pd.power_off = rpmpd_power_off;
>  		rpmpds[i]->pd.power_on = rpmpd_power_on;
> +		rpmpds[i]->pd.set_performance_state = rpmpd_set_performance;
> +		rpmpds[i]->pd.get_performance_state = rpmpd_get_performance;
>  		pm_genpd_init(&rpmpds[i]->pd, NULL, true);
>  
>  		data->domains[i] = &rpmpds[i]->pd;

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH 4/7] soc: qcom: rpmpd: Add support for get/set performance state

[Rajendra Nayak]

On 03/16/2018 10:05 AM, Viresh Kumar wrote:
> On 16-03-18, 09:38, Rajendra Nayak wrote:
>> With genpd now expecting powerdomain drivers supporting performance
>> state to support get/set performance state callbacks, add support for it
>> in the rpmpd driver.
>>
>> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>> ---
>>  drivers/soc/qcom/rpmpd.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 42 insertions(+)
>>
>> diff --git a/drivers/soc/qcom/rpmpd.c b/drivers/soc/qcom/rpmpd.c
>> index c8754d867c33..4058c5b450c6 100644
>> --- a/drivers/soc/qcom/rpmpd.c
>> +++ b/drivers/soc/qcom/rpmpd.c
>> @@ -14,6 +14,7 @@
>>  #include <linux/of.h>
>>  #include <linux/of_device.h>
>>  #include <linux/platform_device.h>
>> +#include <linux/pm_opp.h>
>>  #include <linux/soc/qcom/smd-rpm.h>
>>  
>>  #include <dt-bindings/mfd/qcom-rpm.h>
>> @@ -29,6 +30,8 @@
>>  #define KEY_ENABLE		0x6e657773 /* swen */
>>  #define KEY_FLOOR_CORNER	0x636676   /* vfc */
>>  
>> +#define MAX_RPMPD_STATE		6
>> +
>>  #define DEFINE_RPMPD_CORN_SMPA(_platform, _name, _active, r_id)		\
>>  	static struct rpmpd _platform##_##_active;			\
>>  	static struct rpmpd _platform##_##_name = {			\
>> @@ -222,6 +225,43 @@ static int rpmpd_power_off(struct generic_pm_domain *domain)
>>  	return ret;
>>  }
>>  
>> +static int rpmpd_set_performance(struct generic_pm_domain *domain,
>> +			     unsigned int state)
>> +{
>> +	int ret = 0;
>> +	struct rpmpd *pd = domain_to_rpmpd(domain);
>> +
>> +	mutex_lock(&rpmpd_lock);
>> +
>> +	if (state > MAX_RPMPD_STATE)
>> +		goto out;
>> +
>> +	pd->corner = state;
>> +
>> +	if (!pd->enabled && (pd->key != KEY_FLOOR_CORNER))
>> +		goto out;
>> +
>> +	ret = rpmpd_aggregate_corner(pd);
>> +
>> +out:
>> +	mutex_unlock(&rpmpd_lock);
>> +
>> +	return ret;
>> +}
>> +
>> +static unsigned int rpmpd_get_performance(struct generic_pm_domain *genpd,
>> +					  struct dev_pm_opp *opp)
>> +{
>> +	struct device_node *np;
>> +	unsigned int corner = 0;
>> +
>> +	np = dev_pm_opp_get_of_node(opp);
>> +	of_property_read_u32(np, "qcom,level", &corner);
> 
> Don't we want to error out or do something else in case of an error ?

yes, I missed the error checks, will add.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[PATCH 5/7] arm64: dts: msm8996: Add rpmpd device node

[Rajendra Nayak]

Add rpmpd device node and its OPP table

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 arch/arm64/boot/dts/qcom/msm8996.dtsi | 46 +++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 0a6f7952bbb1..43757a078146 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -297,6 +297,52 @@
 				#clock-cells = <1>;
 			};
 
+			rpmpd: qcom,rpmpd {
+				compatible = "qcom,rpmpd-msm8996";
+				#power-domain-cells = <1>;
+				operating-points-v2 = <&rpmpd_opp_table>, /* cx */
+						      <&rpmpd_opp_table>, /* cx_ao */
+						      <&rpmpd_opp_table>, /* cx_vfc */
+						      <&rpmpd_opp_table>, /* mx */
+						      <&rpmpd_opp_table>, /* mx_ao */
+						      <&rpmpd_opp_table>, /* sscx */
+						      <&rpmpd_opp_table>; /* sscx_vfc */
+			};
+
+			rpmpd_opp_table: opp-table {
+				compatible = "operating-points-v2", "operating-points-v2-qcom";
+
+				rpmpd_opp1: opp@1 {
+					opp-hz = /bits/ 64 <1>;
+					qcom,level = <1>;
+				};
+
+				rpmpd_opp2: opp@2 {
+					opp-hz = /bits/ 64 <2>;
+					qcom,level = <2>;
+				};
+
+				rpmpd_opp3: opp@3 {
+					opp-hz = /bits/ 64 <3>;
+					qcom,level = <3>;
+				};
+
+				rpmpd_opp4: opp@4 {
+					opp-hz = /bits/ 64 <4>;
+					qcom,level = <4>;
+				};
+
+				rpmpd_opp5: opp@5 {
+					opp-hz = /bits/ 64 <5>;
+					qcom,level = <5>;
+				};
+
+				rpmpd_opp6: opp@6 {
+					opp-hz = /bits/ 64 <6>;
+					qcom,level = <6>;
+				};
+			};
+
 			pm8994-regulators {
 				compatible = "qcom,rpm-pm8994-regulators";
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 5/7] arm64: dts: msm8996: Add rpmpd device node

[Viresh Kumar]

On 16-03-18, 09:38, Rajendra Nayak wrote:
> Add rpmpd device node and its OPP table
> 
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/msm8996.dtsi | 46 +++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> index 0a6f7952bbb1..43757a078146 100644
> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> @@ -297,6 +297,52 @@
>  				#clock-cells = <1>;
>  			};
>  
> +			rpmpd: qcom,rpmpd {
> +				compatible = "qcom,rpmpd-msm8996";
> +				#power-domain-cells = <1>;
> +				operating-points-v2 = <&rpmpd_opp_table>, /* cx */
> +						      <&rpmpd_opp_table>, /* cx_ao */
> +						      <&rpmpd_opp_table>, /* cx_vfc */
> +						      <&rpmpd_opp_table>, /* mx */
> +						      <&rpmpd_opp_table>, /* mx_ao */
> +						      <&rpmpd_opp_table>, /* sscx */
> +						      <&rpmpd_opp_table>; /* sscx_vfc */
> +			};
> +
> +			rpmpd_opp_table: opp-table {
> +				compatible = "operating-points-v2", "operating-points-v2-qcom";
> +
> +				rpmpd_opp1: opp@1 {
> +					opp-hz = /bits/ 64 <1>;

Actually this (magic values) isn't allowed right now. I will send a
patchset to make this property optional later on separately (once the
other series is accepted).

> +					qcom,level = <1>;
> +				};
> +
> +				rpmpd_opp2: opp@2 {
> +					opp-hz = /bits/ 64 <2>;
> +					qcom,level = <2>;
> +				};
> +
> +				rpmpd_opp3: opp@3 {
> +					opp-hz = /bits/ 64 <3>;
> +					qcom,level = <3>;
> +				};
> +
> +				rpmpd_opp4: opp@4 {
> +					opp-hz = /bits/ 64 <4>;
> +					qcom,level = <4>;
> +				};
> +
> +				rpmpd_opp5: opp@5 {
> +					opp-hz = /bits/ 64 <5>;
> +					qcom,level = <5>;
> +				};
> +
> +				rpmpd_opp6: opp@6 {
> +					opp-hz = /bits/ 64 <6>;
> +					qcom,level = <6>;
> +				};
> +			};
> +
>  			pm8994-regulators {
>  				compatible = "qcom,rpm-pm8994-regulators";
>  

Everything else is fine. After you remove the opp-hz property from
this table:

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH 5/7] arm64: dts: msm8996: Add rpmpd device node

[Stephen Boyd]

Quoting Rajendra Nayak (2018-03-15 21:08:22)
> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> index 0a6f7952bbb1..43757a078146 100644
> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> @@ -297,6 +297,52 @@
>                                 #clock-cells = <1>;
>                         };
>  
> +                       rpmpd: qcom,rpmpd {

power-controller? power-domain-controller? power-domains? Or something
like that.

> +                               compatible = "qcom,rpmpd-msm8996";
> +                               #power-domain-cells = <1>;
> +                               operating-points-v2 = <&rpmpd_opp_table>, /* cx */
> +                                                     <&rpmpd_opp_table>, /* cx_ao */
> +                                                     <&rpmpd_opp_table>, /* cx_vfc */
> +                                                     <&rpmpd_opp_table>, /* mx */
> +                                                     <&rpmpd_opp_table>, /* mx_ao */
> +                                                     <&rpmpd_opp_table>, /* sscx */
> +                                                     <&rpmpd_opp_table>; /* sscx_vfc */
> +                       };
> +
> +                       rpmpd_opp_table: opp-table {

This should go into the root of the tree? Otherwise it may be populated
by the RPMh platform populate code which would be odd. We should go and
update the platform populate code to always ignore operating-points-v2
compatible nodes too.

> +                               compatible = "operating-points-v2", "operating-points-v2-qcom";

This is backwards? I thought more specific compatible went first.

Re: [PATCH 5/7] arm64: dts: msm8996: Add rpmpd device node

[Viresh Kumar]

On 09-04-18, 09:03, Stephen Boyd wrote:
> We should go and
> update the platform populate code to always ignore operating-points-v2
> compatible nodes too.

Will do that.

-- 
viresh

Re: [PATCH 5/7] arm64: dts: msm8996: Add rpmpd device node

[Rajendra Nayak]

On 04/09/2018 09:33 PM, Stephen Boyd wrote:
> Quoting Rajendra Nayak (2018-03-15 21:08:22)
>> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
>> index 0a6f7952bbb1..43757a078146 100644
>> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
>> @@ -297,6 +297,52 @@
>>                                 #clock-cells = <1>;
>>                         };
>>  
>> +                       rpmpd: qcom,rpmpd {
> 
> power-controller? power-domain-controller? power-domains? Or something
> like that.
> 
>> +                               compatible = "qcom,rpmpd-msm8996";
>> +                               #power-domain-cells = <1>;
>> +                               operating-points-v2 = <&rpmpd_opp_table>, /* cx */
>> +                                                     <&rpmpd_opp_table>, /* cx_ao */
>> +                                                     <&rpmpd_opp_table>, /* cx_vfc */
>> +                                                     <&rpmpd_opp_table>, /* mx */
>> +                                                     <&rpmpd_opp_table>, /* mx_ao */
>> +                                                     <&rpmpd_opp_table>, /* sscx */
>> +                                                     <&rpmpd_opp_table>; /* sscx_vfc */
>> +                       };
>> +
>> +                       rpmpd_opp_table: opp-table {
> 
> This should go into the root of the tree? Otherwise it may be populated
> by the RPMh platform populate code which would be odd. We should go and
> update the platform populate code to always ignore operating-points-v2
> compatible nodes too.
> 
>> +                               compatible = "operating-points-v2", "operating-points-v2-qcom";
> 
> This is backwards? I thought more specific compatible went first.

thanks for the review, will fixup all of these when I respin.


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[PATCH 6/7] mmc: sdhci-msm: Adapt the driver to use OPPs to set clocks/performance state

[Rajendra Nayak]

SDHCI driver needs to set a performance state along with scaling its
clocks. Modify the driver to use the newly introduced powerdomain
performance state based OPPs to scale clocks as well as set an
appropriate powerdomain performance state.

The patch also adds OPPs for sdhci device on msm8996.

On platforms which don't specify an OPP table the driver falls
back to just setting the clock rates (as is done today)

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Cc: linux-mmc@vger.kernel.org
---
 arch/arm64/boot/dts/qcom/msm8996.dtsi | 41 +++++++++++++++++++++++++
 drivers/clk/qcom/gcc-msm8996.c        |  8 ++---
 drivers/mmc/host/sdhci-msm.c          | 57 +++++++++++++++++++++++++++++------
 3 files changed, 92 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 43757a078146..e880a8433596 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -569,8 +569,49 @@
 			 <&gcc GCC_SDCC2_APPS_CLK>,
 			 <&xo_board>;
 			 bus-width = <4>;
+			power-domains = <&rpmpd 0>;
+			operating-points-v2 = <&sdhc_opp_table>;
 		 };
 
+		sdhc_opp_table: opp_table {
+			compatible = "operating-points-v2";
+
+			opp@144000 {
+				opp-hz = /bits/ 64 <144000>;
+				required-opp = <&rpmpd_opp1>;
+			};
+
+			opp@4000000 {
+				opp-hz = /bits/ 64 <400000>;
+				required-opp = <&rpmpd_opp1>;
+			};
+
+			opp@20000000 {
+				opp-hz = /bits/ 64 <20000000>;
+				required-opp = <&rpmpd_opp2>;
+			};
+
+			opp@25000000 {
+				opp-hz = /bits/ 64 <25000000>;
+				required-opp = <&rpmpd_opp2>;
+			};
+
+			opp@50000000 {
+				opp-hz = /bits/ 64 <50000000>;
+				required-opp = <&rpmpd_opp2>;
+			};
+
+			opp@100000000 {
+				opp-hz = /bits/ 64 <100000000>;
+				required-opp = <&rpmpd_opp3>;
+			};
+
+			opp@200000000 {
+				opp-hz = /bits/ 64 <200000000>;
+				required-opp = <&rpmpd_opp3>;
+			};
+		};
+
 		msmgpio: pinctrl@1010000 {
 			compatible = "qcom,msm8996-pinctrl";
 			reg = <0x01010000 0x300000>;
diff --git a/drivers/clk/qcom/gcc-msm8996.c b/drivers/clk/qcom/gcc-msm8996.c
index 5d7451209206..fc90dbbdcd70 100644
--- a/drivers/clk/qcom/gcc-msm8996.c
+++ b/drivers/clk/qcom/gcc-msm8996.c
@@ -464,7 +464,7 @@ static struct clk_rcg2 sdcc1_apps_clk_src = {
 		.name = "sdcc1_apps_clk_src",
 		.parent_names = gcc_xo_gpll0_gpll4_gpll0_early_div,
 		.num_parents = 4,
-		.ops = &clk_rcg2_floor_ops,
+		.ops = &clk_rcg2_ops,
 	},
 };
 
@@ -509,7 +509,7 @@ static struct clk_rcg2 sdcc2_apps_clk_src = {
 		.name = "sdcc2_apps_clk_src",
 		.parent_names = gcc_xo_gpll0_gpll4,
 		.num_parents = 3,
-		.ops = &clk_rcg2_floor_ops,
+		.ops = &clk_rcg2_ops,
 	},
 };
 
@@ -523,7 +523,7 @@ static struct clk_rcg2 sdcc3_apps_clk_src = {
 		.name = "sdcc3_apps_clk_src",
 		.parent_names = gcc_xo_gpll0_gpll4,
 		.num_parents = 3,
-		.ops = &clk_rcg2_floor_ops,
+		.ops = &clk_rcg2_ops,
 	},
 };
 
@@ -547,7 +547,7 @@ static struct clk_rcg2 sdcc4_apps_clk_src = {
 		.name = "sdcc4_apps_clk_src",
 		.parent_names = gcc_xo_gpll0,
 		.num_parents = 2,
-		.ops = &clk_rcg2_floor_ops,
+		.ops = &clk_rcg2_ops,
 	},
 };
 
diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index c283291db705..8e6abc0a23c7 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -21,6 +21,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/slab.h>
 #include <linux/iopoll.h>
+#include <linux/pm_opp.h>
 
 #include "sdhci-pltfm.h"
 
@@ -137,6 +138,7 @@ struct sdhci_msm_host {
 	struct clk *bus_clk;	/* SDHC bus voter clock */
 	struct clk *xo_clk;	/* TCXO clk needed for FLL feature of cm_dll*/
 	struct clk_bulk_data bulk_clks[4]; /* core, iface, cal, sleep clocks */
+	struct opp_table *opp_table;
 	unsigned long clk_rate;
 	struct mmc_host *mmc;
 	bool use_14lpp_dll_reset;
@@ -150,7 +152,7 @@ struct sdhci_msm_host {
 	bool pwr_irq_flag;
 };
 
-static unsigned int msm_get_clock_rate_for_bus_mode(struct sdhci_host *host,
+static long unsigned int msm_get_clock_rate_for_bus_mode(struct sdhci_host *host,
 						    unsigned int clock)
 {
 	struct mmc_ios ios = host->mmc->ios;
@@ -176,18 +178,37 @@ static void msm_set_clock_rate_for_bus_mode(struct sdhci_host *host,
 	struct mmc_ios curr_ios = host->mmc->ios;
 	struct clk *core_clk = msm_host->bulk_clks[0].clk;
 	int rc;
+	struct device *dev = &msm_host->pdev->dev;
+	struct dev_pm_opp *opp;
+	long unsigned int freq;
+
+	if (msm_host->opp_table) {
+		freq = msm_get_clock_rate_for_bus_mode(host, clock);
+		opp = dev_pm_opp_find_freq_floor(dev, &freq);
+		if (IS_ERR(opp)) {
+			pr_err("%s: failed to find OPP for %u at timing %d\n",
+				mmc_hostname(host->mmc), clock, curr_ios.timing);
+			return;
+		}
+		rc = dev_pm_opp_set_rate(dev, freq);
+		if (rc)
+			pr_err("%s: error in setting opp\n", __func__);
 
-	clock = msm_get_clock_rate_for_bus_mode(host, clock);
-	rc = clk_set_rate(core_clk, clock);
-	if (rc) {
-		pr_err("%s: Failed to set clock at rate %u at timing %d\n",
-		       mmc_hostname(host->mmc), clock,
-		       curr_ios.timing);
-		return;
+		msm_host->clk_rate = freq;
+	} else {
+		clock = msm_get_clock_rate_for_bus_mode(host, clock);
+		rc = clk_set_rate(core_clk, clock);
+		if (rc) {
+			pr_err("%s: Failed to set clock at rate %u at timing %d\n",
+			       mmc_hostname(host->mmc), clock,
+			       curr_ios.timing);
+			return;
+		}
+		msm_host->clk_rate = clock;
 	}
-	msm_host->clk_rate = clock;
+
 	pr_debug("%s: Setting clock at rate %lu at timing %d\n",
-		 mmc_hostname(host->mmc), clk_get_rate(core_clk),
+		 mmc_hostname(host->mmc), msm_host->clk_rate,
 		 curr_ios.timing);
 }
 
@@ -1519,6 +1540,16 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 		goto clk_disable;
 	}
 
+	/* Set up the OPP table if it exists */
+	msm_host->opp_table = dev_pm_opp_set_clkname(&pdev->dev, "core");
+
+	ret = dev_pm_opp_of_add_table(&pdev->dev);
+	if (ret) {
+		dev_warn(&pdev->dev, "%s: No OPP table specified\n", __func__);
+		dev_pm_opp_put_clkname(msm_host->opp_table);
+		msm_host->opp_table = NULL;
+	}
+
 	pm_runtime_get_noresume(&pdev->dev);
 	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
@@ -1540,6 +1571,9 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 	pm_runtime_disable(&pdev->dev);
 	pm_runtime_set_suspended(&pdev->dev);
 	pm_runtime_put_noidle(&pdev->dev);
+	dev_pm_opp_of_remove_table(&pdev->dev);
+	if (msm_host->opp_table)
+		dev_pm_opp_put_clkname(msm_host->opp_table);
 clk_disable:
 	clk_bulk_disable_unprepare(ARRAY_SIZE(msm_host->bulk_clks),
 				   msm_host->bulk_clks);
@@ -1564,6 +1598,9 @@ static int sdhci_msm_remove(struct platform_device *pdev)
 	pm_runtime_get_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 	pm_runtime_put_noidle(&pdev->dev);
+	dev_pm_opp_of_remove_table(&pdev->dev);
+	if (msm_host->opp_table)
+		dev_pm_opp_put_clkname(msm_host->opp_table);
 
 	clk_bulk_disable_unprepare(ARRAY_SIZE(msm_host->bulk_clks),
 				   msm_host->bulk_clks);
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 6/7] mmc: sdhci-msm: Adapt the driver to use OPPs to set clocks/performance state

[Viresh Kumar]

On 16-03-18, 09:38, Rajendra Nayak wrote:
> @@ -1540,6 +1571,9 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  	pm_runtime_disable(&pdev->dev);
>  	pm_runtime_set_suspended(&pdev->dev);
>  	pm_runtime_put_noidle(&pdev->dev);
> +	dev_pm_opp_of_remove_table(&pdev->dev);

You can't do this if there is no OPP table. Probably you should just
make it part of the below if block.

> +	if (msm_host->opp_table)
> +		dev_pm_opp_put_clkname(msm_host->opp_table);
>  clk_disable:
>  	clk_bulk_disable_unprepare(ARRAY_SIZE(msm_host->bulk_clks),
>  				   msm_host->bulk_clks);
> @@ -1564,6 +1598,9 @@ static int sdhci_msm_remove(struct platform_device *pdev)
>  	pm_runtime_get_sync(&pdev->dev);
>  	pm_runtime_disable(&pdev->dev);
>  	pm_runtime_put_noidle(&pdev->dev);
> +	dev_pm_opp_of_remove_table(&pdev->dev);

And this too.

> +	if (msm_host->opp_table)
> +		dev_pm_opp_put_clkname(msm_host->opp_table);
>  
>  	clk_bulk_disable_unprepare(ARRAY_SIZE(msm_host->bulk_clks),
>  				   msm_host->bulk_clks);

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

[PATCH 7/7] soc: qcom: rpmpd: Add a max vote on all corners at init

[Rajendra Nayak]

As we move from no clients/consumers in kernel voting on corners,
to *some* voting and some not voting, we might end up in a situation
where the clients which remove votes can adversly impact others
who still don't have a way to vote.

To avoid this situation, have a max vote on all corners at init.
This should/can be removed once we have all clients moved to
be able to vote/unvote for themselves.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 drivers/soc/qcom/rpmpd.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/soc/qcom/rpmpd.c b/drivers/soc/qcom/rpmpd.c
index 4058c5b450c6..ebdcf9398441 100644
--- a/drivers/soc/qcom/rpmpd.c
+++ b/drivers/soc/qcom/rpmpd.c
@@ -304,6 +304,15 @@ static int rpmpd_probe(struct platform_device *pdev)
 		pm_genpd_init(&rpmpds[i]->pd, NULL, true);
 
 		data->domains[i] = &rpmpds[i]->pd;
+
+		/*
+		 * Until we have all consumers voting on corners
+		 * just vote the max corner on all PDs
+		 * This should ideally be *removed* once we have
+		 * all (most) consumers being able to vote
+		 */
+		rpmpd_set_performance(&rpmpds[i]->pd, MAX_RPMPD_STATE);
+		rpmpd_power_on(&rpmpds[i]->pd);
 	}
 
 	return of_genpd_add_provider_onecell(pdev->dev.of_node, data);
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 7/7] soc: qcom: rpmpd: Add a max vote on all corners at init

[Viresh Kumar]

On 16-03-18, 09:38, Rajendra Nayak wrote:
> As we move from no clients/consumers in kernel voting on corners,
> to *some* voting and some not voting, we might end up in a situation
> where the clients which remove votes can adversly impact others
> who still don't have a way to vote.
> 
> To avoid this situation, have a max vote on all corners at init.
> This should/can be removed once we have all clients moved to
> be able to vote/unvote for themselves.
> 
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---
>  drivers/soc/qcom/rpmpd.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/soc/qcom/rpmpd.c b/drivers/soc/qcom/rpmpd.c
> index 4058c5b450c6..ebdcf9398441 100644
> --- a/drivers/soc/qcom/rpmpd.c
> +++ b/drivers/soc/qcom/rpmpd.c
> @@ -304,6 +304,15 @@ static int rpmpd_probe(struct platform_device *pdev)
>  		pm_genpd_init(&rpmpds[i]->pd, NULL, true);
>  
>  		data->domains[i] = &rpmpds[i]->pd;
> +
> +		/*
> +		 * Until we have all consumers voting on corners
> +		 * just vote the max corner on all PDs
> +		 * This should ideally be *removed* once we have
> +		 * all (most) consumers being able to vote
> +		 */
> +		rpmpd_set_performance(&rpmpds[i]->pd, MAX_RPMPD_STATE);
> +		rpmpd_power_on(&rpmpds[i]->pd);
>  	}
>  
>  	return of_genpd_add_provider_onecell(pdev->dev.of_node, data);

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh