[v4] regulator: qcom-saw: Add support for SAW regulators

Message ID 1455023549-30836-1-git-send-email-georgi.djakov@linaro.org
State New
Headers show

Commit Message

Georgi Djakov Feb. 9, 2016, 1:12 p.m.
The SAW (Subsystem Power Manager and Adaptive Voltage Scaling Wrapper)
is part of the SPM subsystem. It is a hardware block in the Qualcomm
chipsets that regulates the power to the CPU cores on platform such as
apq8064, msm8974, apq8084 and others.

Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>

---

Changes since v3 (https://lkml.org/lkml/2016/2/3/829)
 * Add MFD dependency to Kconfig
 * Rename SAW2 to SAW as we may support other SAW generations than just 2
 * Increase timeout to 100us

Changes since v2 (https://lkml.org/lkml/2016/1/28/481)
 * Address the comments from Mark. Thanks!
 - Implement regulator_get_voltage_sel() instead of regulator_get_voltage()
 - Add cpu_relax() in the loop
 - Specify ramp_delay
 - Drop the legacy "regulator-name" property

Changes since v1 (https://lkml.org/lkml/2015/12/18/629)
 * Move into a separate regulator driver

 .../bindings/regulator/qcom,saw-regulator.txt      |   31 +++
 drivers/regulator/Kconfig                          |   12 +
 drivers/regulator/Makefile                         |    1 +
 drivers/regulator/qcom_saw-regulator.c             |  229 ++++++++++++++++++++
 4 files changed, 273 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/qcom,saw-regulator.txt
 create mode 100644 drivers/regulator/qcom_saw-regulator.c

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Lina Iyer Feb. 9, 2016, 10:21 p.m. | #1
On Tue, Feb 09 2016 at 06:13 -0700, Georgi Djakov wrote:
>The SAW (Subsystem Power Manager and Adaptive Voltage Scaling Wrapper)

>is part of the SPM subsystem. It is a hardware block in the Qualcomm

>chipsets that regulates the power to the CPU cores on platform such as

>apq8064, msm8974, apq8084 and others.

>

>Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>

>---

>

>Changes since v3 (https://lkml.org/lkml/2016/2/3/829)

> * Add MFD dependency to Kconfig

> * Rename SAW2 to SAW as we may support other SAW generations than just 2

> * Increase timeout to 100us

>

>Changes since v2 (https://lkml.org/lkml/2016/1/28/481)

> * Address the comments from Mark. Thanks!

> - Implement regulator_get_voltage_sel() instead of regulator_get_voltage()

> - Add cpu_relax() in the loop

> - Specify ramp_delay

> - Drop the legacy "regulator-name" property

>

>Changes since v1 (https://lkml.org/lkml/2015/12/18/629)

> * Move into a separate regulator driver

>

> .../bindings/regulator/qcom,saw-regulator.txt      |   31 +++

> drivers/regulator/Kconfig                          |   12 +

> drivers/regulator/Makefile                         |    1 +

> drivers/regulator/qcom_saw-regulator.c             |  229 ++++++++++++++++++++

> 4 files changed, 273 insertions(+)

> create mode 100644 Documentation/devicetree/bindings/regulator/qcom,saw-regulator.txt

> create mode 100644 drivers/regulator/qcom_saw-regulator.c

>

>diff --git a/Documentation/devicetree/bindings/regulator/qcom,saw-regulator.txt b/Documentation/devicetree/bindings/regulator/qcom,saw-regulator.txt

>new file mode 100644

>index 000000000000..977fec08b2ae

>--- /dev/null

>+++ b/Documentation/devicetree/bindings/regulator/qcom,saw-regulator.txt

>@@ -0,0 +1,31 @@

>+Qualcomm SAW Regulators

>+

>+SAW (Subsystem Power Manager and Adaptive Voltage Scaling Wrapper) is a hardware

>+block in the Qualcomm chipsets that regulates the power to the CPU cores on devices

>+such as APQ8064, MSM8974, APQ8084 and others.

>+

>+- compatible:

>+	Usage: required

>+	Value type: <string>

>+	Definition: must be one of:

>+			"qcom,apq8064-saw2-v1.1-regulator"

>+

>+Example:

>+                saw0: power-controller@2089000 {

>+			compatible = "qcom,apq8064-saw2-v1.1-cpu", "qcom,saw2", "syscon", "simple-mfd";

>+			reg = <0x02089000 0x1000>, <0x02009000 0x1000>;

>+			#address-cells = <1>;

>+			#size-cells = <1>;

>+

>+			saw0_regulator: regulator@2089000 {

>+				compatible = "qcom,apq8064-saw2-v1.1-regulator";

>+				regulator-always-on;

>+				regulator-min-microvolt = <825000>;

>+				regulator-max-microvolt = <1250000>;

>+			};

>+		};

>+

>+

>+		&CPU0 {

>+			cpu-supply = <&saw0_regulator>;

>+		};

>diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig

>index 74a6354eaefa..6d5f4fce1d75 100644

>--- a/drivers/regulator/Kconfig

>+++ b/drivers/regulator/Kconfig

>@@ -558,6 +558,18 @@ config REGULATOR_QCOM_RPM

> 	  Qualcomm RPM as a module. The module will be named

> 	  "qcom_rpm-regulator".

>

>+config REGULATOR_QCOM_SAW

>+	tristate "Qualcomm SAW regulator driver"

>+	depends on (ARCH_QCOM || COMPILE_TEST) && MFD_SYSCON

>+	help

>+	  If you say yes to this option, support will be included for the

>+	  regulators providing power to the CPU cores on devices such as

>+	  APQ8064.

>+

>+	  Say M here if you want to include support for the CPU core voltage

>+	  regulators as a module. The module will be named

>+	  "qcom_saw-regulator".

>+

> config REGULATOR_QCOM_SMD_RPM

> 	tristate "Qualcomm SMD based RPM regulator driver"

> 	depends on QCOM_SMD_RPM

>diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile

>index 348cfd727350..75a0b4a8f1b2 100644

>--- a/drivers/regulator/Makefile

>+++ b/drivers/regulator/Makefile

>@@ -64,6 +64,7 @@ obj-$(CONFIG_REGULATOR_MC13XXX_CORE) +=  mc13xxx-regulator-core.o

> obj-$(CONFIG_REGULATOR_MT6311) += mt6311-regulator.o

> obj-$(CONFIG_REGULATOR_MT6397)	+= mt6397-regulator.o

> obj-$(CONFIG_REGULATOR_QCOM_RPM) += qcom_rpm-regulator.o

>+obj-$(CONFIG_REGULATOR_QCOM_SAW)+= qcom_saw-regulator.o

> obj-$(CONFIG_REGULATOR_QCOM_SMD_RPM) += qcom_smd-regulator.o

> obj-$(CONFIG_REGULATOR_QCOM_SPMI) += qcom_spmi-regulator.o

> obj-$(CONFIG_REGULATOR_PALMAS) += palmas-regulator.o

>diff --git a/drivers/regulator/qcom_saw-regulator.c b/drivers/regulator/qcom_saw-regulator.c

>new file mode 100644

>index 000000000000..c800f16adaf0

>--- /dev/null

>+++ b/drivers/regulator/qcom_saw-regulator.c

>@@ -0,0 +1,229 @@

>+/*

>+ * Copyright (c) 2016, Linaro Limited. All rights reserved.

>+ *

>+ * This program is free software; you can redistribute it and/or modify

>+ * it under the terms of the GNU General Public License version 2 and

>+ * only version 2 as published by the Free Software Foundation.

>+ *

>+ * This program is distributed in the hope that it will be useful,

>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of

>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the

>+ * GNU General Public License for more details.

>+ */

>+

>+#include <linux/delay.h>

>+#include <linux/kernel.h>

>+#include <linux/mfd/syscon.h>

>+#include <linux/module.h>

>+#include <linux/of.h>

>+#include <linux/of_platform.h>

>+#include <linux/platform_device.h>

>+#include <linux/regmap.h>

>+#include <linux/regulator/driver.h>

>+#include <linux/regulator/of_regulator.h>

>+

>+#define	SPM_REG_STS_1			0x10

>+#define	SPM_REG_VCTL			0x14

>+#define	SPM_REG_PMIC_DATA_0		0x28

>+#define	SPM_REG_PMIC_DATA_1		0x2c

>+#define	SPM_REG_RST			0x30

>+

These register offsets are SoC specific. You may want to follow the model
of drivers/soc/qcom/spm.c in getting register offsets.

While I see that you are only supporting APQ8064 with this patch, you
probably would want to think a bit far ahead. To support any other QCOM
SoC, you would need extensive changes.

>+struct saw_vreg {

>+	struct device		*dev;

>+	struct regmap		*regmap;

>+	struct regulator_desc	rdesc;

>+	struct regulator_dev	*rdev;

>+	unsigned int		sel;

>+};

>+

>+struct spm_vlevel_data {

>+	struct saw_vreg *vreg;

>+	unsigned int sel;

>+};

>+

>+static int saw_regulator_get_voltage_sel(struct regulator_dev *rdev)

>+{

>+	struct saw_vreg *vreg = rdev_get_drvdata(rdev);

>+

>+	return vreg->sel;

>+}

>+

>+static void smp_set_vdd(void *data)

>+{

>+	struct spm_vlevel_data *vdata = (struct spm_vlevel_data *)data;

>+	struct saw_vreg *vreg = vdata->vreg;

>+	unsigned long new_sel = vdata->sel;

>+	u32 val, new_val;

>+	u32 vctl, data0, data1;

>+	unsigned long timeout;

>+

>+	if (vreg->sel == new_sel)

>+		return;

>+

>+	regmap_read(vreg->regmap, SPM_REG_VCTL, &vctl);

>+	regmap_read(vreg->regmap, SPM_REG_PMIC_DATA_0, &data0);

>+	regmap_read(vreg->regmap, SPM_REG_PMIC_DATA_1, &data1);

>+

>+	/* select the band */

>+	val = 0x80 | new_sel;

>+

>+	vctl &= ~0xff;

>+	vctl |= val;

>+

>+	data0 &= ~0xff;

>+	data0 |= val;

>+

>+	data1 &= ~0x3f;

>+	data1 |= val & 0x3f;

>+	data1 &= ~0x3f0000;

>+	data1 |= ((val & 0x3f) << 16);

>+

>+	regmap_write(vreg->regmap, SPM_REG_RST, 1);

>+	regmap_write(vreg->regmap, SPM_REG_VCTL, vctl);

>+	regmap_write(vreg->regmap, SPM_REG_PMIC_DATA_0, data0);

>+	regmap_write(vreg->regmap, SPM_REG_PMIC_DATA_1, data1);

>+

>+	timeout = jiffies + usecs_to_jiffies(100);

>+	do {

>+		regmap_read(vreg->regmap, SPM_REG_STS_1, &new_val);

>+		new_val &= 0xff;

>+		if (new_val == val) {

>+			vreg->sel = new_sel;

>+			return;

>+		}

>+

>+		cpu_relax();

>+

>+	} while (time_before(jiffies, timeout));

>+

>+	pr_err("%s: Voltage not changed: %#x\n", __func__, new_val);

>+}

>+

>+static int saw_regulator_set_voltage_sel(struct regulator_dev *rdev,

>+					 unsigned selector)

>+{

>+	struct saw_vreg *vreg = rdev_get_drvdata(rdev);

>+	struct spm_vlevel_data data;

>+	int cpu = rdev_get_id(rdev);

>+

>+	data.vreg = vreg;

>+	data.sel = selector;

>+

>+	return smp_call_function_single(cpu, smp_set_vdd, &data, true);

>+}

>+

>+static struct regulator_ops saw_regulator_ops = {

>+	.list_voltage = regulator_list_voltage_linear_range,

>+	.set_voltage_sel = saw_regulator_set_voltage_sel,

>+	.get_voltage_sel = saw_regulator_get_voltage_sel,

>+	.set_voltage_time_sel = regulator_set_voltage_time_sel,

>+};

>+

>+static struct regulator_desc saw_regulator = {

>+	.owner = THIS_MODULE,

>+	.type = REGULATOR_VOLTAGE,

>+	.ops  = &saw_regulator_ops,

>+	.linear_ranges = (struct regulator_linear_range[]) {

>+		REGULATOR_LINEAR_RANGE(700000, 0, 56, 12500),

>+	},

>+	.n_linear_ranges = 1,

>+	.n_voltages = 57,

>+	.ramp_delay = 1250,

>+};

>+

>+static struct saw_vreg *saw_get_drv(struct platform_device *pdev,

>+				    int *vreg_cpu)

>+{

>+	struct saw_vreg *vreg = NULL;

>+	struct device_node *cpu_node, *saw_node;

>+	int cpu;

>+	bool found;

>+

>+	for_each_possible_cpu(cpu) {

>+		cpu_node = of_cpu_device_node_get(cpu);

>+		if (!cpu_node)

>+			continue;

>+		saw_node = of_parse_phandle(cpu_node, "qcom,saw", 0);

>+		found = (saw_node == pdev->dev.of_node->parent);

>+		of_node_put(saw_node);

>+		of_node_put(cpu_node);

>+		if (found)

>+			break;

>+	}

>+

>+	if (found) {

>+		vreg = devm_kzalloc(&pdev->dev, sizeof(*vreg), GFP_KERNEL);

>+		if (vreg)

>+			*vreg_cpu = cpu;

>+	}

>+

>+	return vreg;

>+}

>+

>+static const struct of_device_id qcom_saw_regulator_match[] = {

>+	{ .compatible = "qcom,apq8064-saw2-v1.1-regulator" },

>+	{ }

>+};

>+MODULE_DEVICE_TABLE(of, qcom_saw_regulator_match);

>+

>+static int qcom_saw_regulator_probe(struct platform_device *pdev)

>+{

>+	struct device *dev = &pdev->dev;

>+	struct device_node *np = dev->of_node;

>+	struct device_node *saw_np;

>+	struct saw_vreg *vreg;

>+	struct regulator_config config = { };

>+	int ret = 0, cpu = 0;

>+	char name[] = "kraitXX";

>+

>+	vreg = saw_get_drv(pdev, &cpu);

>+	if (!vreg)

>+		return -EINVAL;

>+

>+	saw_np = of_get_parent(np);

>+	if (!saw_np)

>+		return -ENODEV;

>+

>+	vreg->regmap = syscon_node_to_regmap(saw_np);

>+	of_node_put(saw_np);

>+	if (IS_ERR(config.regmap))

>+		return PTR_ERR(config.regmap);

>+

>+	snprintf(name, sizeof(name), "krait%d", cpu);

>+

>+	config.regmap = vreg->regmap;

>+	config.dev = &pdev->dev;

>+	config.of_node = np;

>+	config.driver_data = vreg;

>+

>+	vreg->rdesc = saw_regulator;

>+	vreg->rdesc.id = cpu;

>+	vreg->rdesc.name = name;

>+	config.init_data = of_get_regulator_init_data(&pdev->dev,

>+						      pdev->dev.of_node,

>+						      &vreg->rdesc);

>+

>+	vreg->rdev = devm_regulator_register(&pdev->dev, &vreg->rdesc, &config);

>+	if (IS_ERR(vreg->rdev)) {

>+		ret = PTR_ERR(vreg->rdev);

>+		dev_err(dev, "failed to register SAW regulator: %d\n", ret);

>+		return ret;

>+	}

>+

>+	return 0;

>+}

>+

>+static struct platform_driver qcom_saw_regulator_driver = {

>+	.driver = {

>+		.name = "qcom-saw-regulator",

>+		.of_match_table = qcom_saw_regulator_match,

>+	},

>+	.probe = qcom_saw_regulator_probe,

>+};

>+

>+module_platform_driver(qcom_saw_regulator_driver);

>+

builtin_platform_driver() perhaps ?

>+MODULE_ALIAS("platform:qcom-saw-regulator");

>+MODULE_DESCRIPTION("Qualcomm SAW regulator driver");

>+MODULE_AUTHOR("Georgi Djakov <georgi.djakov@linaro.org>");

>+MODULE_LICENSE("GPL v2");

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Boyd Feb. 10, 2016, 6:36 p.m. | #2
On 02/10, Georgi Djakov wrote:
> Hi Lina,

> Thanks for reviewing.

> 

> On 02/10/2016 12:21 AM, Lina Iyer wrote:

> > On Tue, Feb 09 2016 at 06:13 -0700, Georgi Djakov wrote:

> [..]

> >> +#define    SPM_REG_STS_1            0x10

> >> +#define    SPM_REG_VCTL            0x14

> >> +#define    SPM_REG_PMIC_DATA_0        0x28

> >> +#define    SPM_REG_PMIC_DATA_1        0x2c

> >> +#define    SPM_REG_RST            0x30

> >> +

> > These register offsets are SoC specific. You may want to follow the model

> > of drivers/soc/qcom/spm.c in getting register offsets.

> > 

> > While I see that you are only supporting APQ8064 with this patch, you

> > probably would want to think a bit far ahead. To support any other QCOM

> > SoC, you would need extensive changes.

> > 

> 

> The purpose of this patch it to add support for 8064. Supporting other 

> SoCs requires just read/writing at different offsets. To handle this we

> can convert the above defines to a table containing the offsets for each

> SoC. I don't think these are extensive changes or do i miss something?

> 


In some designs we have to talk to the PMIC with SPMI
transactions to change the mode depending on the voltages. How do
we plan to handle that case where the regulator control is split
between two busses, SPMI and MMIO?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown Feb. 10, 2016, 6:43 p.m. | #3
On Wed, Feb 10, 2016 at 10:36:51AM -0800, Stephen Boyd wrote:
> On 02/10, Georgi Djakov wrote:


> > The purpose of this patch it to add support for 8064. Supporting other 

> > SoCs requires just read/writing at different offsets. To handle this we

> > can convert the above defines to a table containing the offsets for each

> > SoC. I don't think these are extensive changes or do i miss something?


> In some designs we have to talk to the PMIC with SPMI

> transactions to change the mode depending on the voltages. How do

> we plan to handle that case where the regulator control is split

> between two busses, SPMI and MMIO?


Mixed control interfaces are in general a diaster with Linux, I'd
suggest remonstrating with your system designers about doing them but in
this case since it's a syscon presumably you could hang the device off
the SPMI bus in the cases where that's in use.
Mark Brown Feb. 10, 2016, 6:54 p.m. | #4
On Wed, Feb 10, 2016 at 09:42:23AM -0700, Lina Iyer wrote:

> The offsets are the simple part. On other QCOM SoCs the voltage is

> regulated by the SAW attached to the L2 cache controller. They have a

> single rail that supplies power to all the cores and the L2, with

> ability to change the number of phases to match the load. This driver

> supports 8064 which has individual CPU rails, while the rest of the QCOM

> SoC's share a common rail design.


That doesn't seem at all hard to handle, the regulator framework is
intended to cope with regulators with multiple consumers since it's the
common case.
Stephen Boyd Feb. 10, 2016, 7:04 p.m. | #5
On 02/10, Mark Brown wrote:
> On Wed, Feb 10, 2016 at 10:36:51AM -0800, Stephen Boyd wrote:

> > On 02/10, Georgi Djakov wrote:

> 

> > In some designs we have to talk to the PMIC with SPMI

> > transactions to change the mode depending on the voltages. How do

> > we plan to handle that case where the regulator control is split

> > between two busses, SPMI and MMIO?

> 

> Mixed control interfaces are in general a diaster with Linux, I'd

> suggest remonstrating with your system designers about doing them but in

> this case since it's a syscon presumably you could hang the device off

> the SPMI bus in the cases where that's in use.



Agreed about the mixed control interfaces, it's a total nightmare
to deal with. Luckily this problem is going to go away in the
future, but we're stuck with what we have for this generation of
chips.

I don't follow the rest of your mail though. Are you suggesting
that in this case we put the regulator control into the PMIC
regulator driver (qcom_spmi-regulator.c) and then use a
syscon/regmap there to change the voltages inside the MMIO bus?
That may work but we're going to need to update the binding for
the SPMI regulator driver then.

I'm not really excited about the binding we have here either.
We're going to have two places in DT where we've created a
regulator for the same physical regulator. One will be a child of
the SAW node on the MMIO bus, and another will be a child of the
PMIC on the SPMI/SSBI bus. In the end, they're both the same
regulator, so any constraints on one node will need to be applied
to the other node as well.

A final note, 8064 is paired with a PMIC on the SSBI bus. We
don't have an SSBI regulator driver upstream, so we would need
some driver + binding for that regulator style if we want to do
this for 8064.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown Feb. 10, 2016, 7:21 p.m. | #6
On Wed, Feb 10, 2016 at 11:04:36AM -0800, Stephen Boyd wrote:

> I don't follow the rest of your mail though. Are you suggesting

> that in this case we put the regulator control into the PMIC

> regulator driver (qcom_spmi-regulator.c) and then use a

> syscon/regmap there to change the voltages inside the MMIO bus?

> That may work but we're going to need to update the binding for

> the SPMI regulator driver then.


No, why would you want to do that?  I'm saying that if the device has a
SPMI interface make that the primary interface for the driver.
Presumably the SPMI bus abstraction is capable of representing this
fairly directly.

> I'm not really excited about the binding we have here either.

> We're going to have two places in DT where we've created a

> regulator for the same physical regulator. One will be a child of

> the SAW node on the MMIO bus, and another will be a child of the

> PMIC on the SPMI/SSBI bus. In the end, they're both the same

> regulator, so any constraints on one node will need to be applied

> to the other node as well.


Are you saying that this is a problem with the driver that just got
merged?  We got to v4 before I applied the driver...  My understanding
was that this is a driver for a new regulator type not a duplicate
interface for existing regulator.
Stephen Boyd Feb. 10, 2016, 10:46 p.m. | #7
On 02/10, Mark Brown wrote:
> On Wed, Feb 10, 2016 at 11:04:36AM -0800, Stephen Boyd wrote:

> 

> > I don't follow the rest of your mail though. Are you suggesting

> > that in this case we put the regulator control into the PMIC

> > regulator driver (qcom_spmi-regulator.c) and then use a

> > syscon/regmap there to change the voltages inside the MMIO bus?

> > That may work but we're going to need to update the binding for

> > the SPMI regulator driver then.

> 

> No, why would you want to do that?  I'm saying that if the device has a

> SPMI interface make that the primary interface for the driver.

> Presumably the SPMI bus abstraction is capable of representing this

> fairly directly.

> 

> > I'm not really excited about the binding we have here either.

> > We're going to have two places in DT where we've created a

> > regulator for the same physical regulator. One will be a child of

> > the SAW node on the MMIO bus, and another will be a child of the

> > PMIC on the SPMI/SSBI bus. In the end, they're both the same

> > regulator, so any constraints on one node will need to be applied

> > to the other node as well.

> 

> Are you saying that this is a problem with the driver that just got

> merged?  We got to v4 before I applied the driver...  My understanding

> was that this is a driver for a new regulator type not a duplicate

> interface for existing regulator.


Yeah I haven't had the time to properly review this patch. From
what I can tell though, if this driver probes first and then the
spm driver probes second, the CPU could go idle and then wakeup
with a lower voltage than is required.

The hardware works like this: there's a regulator on the PMIC
that's dedicated to the CPU. Let's call this regulator S1. The
PMIC is on the SPMI bus. There's an SPMI master controller inside
the SoC. This controller can be used to perform SPMI transactions
when software does MMIO read/writes or it can be perform
transactions on the behalf of some hardware entity. The
controller is called the "PMIC arbiter" because it's arbitrating
access to the PMIC between all the sw entities (linux, modem,
dsp, etc.) and the hw entities (various SAWs in the system).

Typical control of an SPMI regulator can be seen in the
qcom_spmi-regulator.c driver. That's a standard SPMI bus
regulator driver. The SAWs are "hardwired" to a PMIC regulator
based on some silicon configuration. In this example, the SAW
would be configured to send commands to the PMIC for the S1
regulator. The SAW hardware will basically takes whatever voltage
is written to it in the MMIO space and tacks on an address for S1
before handing that over to the PMIC arbiter to send it to the
PMIC. One way to think of this is that each SAW has its own SPMI
regulator driver for the one regulator it cares about, except we
have to format the payload in the same way that the PMIC would
expect it.

Now this is all pretty much useless hardware if all we care about
is to set voltages from software. Obviously we could just use the
SPMI regulator driver that we already have. It's written for the
bus that the hardware is actually on and it works!

The problem there is that the SAW register is also used during
idle/suspend when software isn't running. The CPU power down
sequence usually turns off the regulator, but it may also change
the voltage to something lower, depending on how deep of
idle/suspend we're trying to achieve. This is all done without
software intervention. When the CPU wakes up due to some
interrupt, the SPM runs the CPU power on sequence which takes
whatever is in the SAW register and sends it off to the PMIC to
restore the CPU voltage. Again, this is all hardware doing this.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Boyd Feb. 12, 2016, 12:17 a.m. | #8
On 02/11, Georgi Djakov wrote:
> On 02/11/2016 12:46 AM, Stephen Boyd wrote:

> > On 02/10, Mark Brown wrote:

> >> On Wed, Feb 10, 2016 at 11:04:36AM -0800, Stephen Boyd wrote:

> >>

> [..]

> >>> I'm not really excited about the binding we have here either.

> >>> We're going to have two places in DT where we've created a

> >>> regulator for the same physical regulator. One will be a child of

> >>> the SAW node on the MMIO bus, and another will be a child of the

> >>> PMIC on the SPMI/SSBI bus. In the end, they're both the same

> >>> regulator, so any constraints on one node will need to be applied

> >>> to the other node as well.

> >>

> >> Are you saying that this is a problem with the driver that just got

> >> merged?  We got to v4 before I applied the driver...  My understanding

> >> was that this is a driver for a new regulator type not a duplicate

> >> interface for existing regulator.

> > 

> [..]

> > 

> > Now this is all pretty much useless hardware if all we care about

> > is to set voltages from software. Obviously we could just use the

> > SPMI regulator driver that we already have. It's written for the

> > bus that the hardware is actually on and it works!

> > 

> 

> 8064 uses SSBI instead of SPMI and we currently do not have any

> existing regulator support upstream yet. So this driver is not

> duplicating any existing regulator. We should decide whether to

> keep this driver or to replace it with a new ssbi-regulator driver

> and bindings instead, where we can avoid the split-bus fun at

> least to some extent. Maybe the latter is the better option?


Yes I think having an ssbi/spmi regulator driver may be a better
approach. The SAW code can monitor the regulator for voltage
changes with a notifier and then stick the restore voltage into
the SAW registers. There's only one sticking point below.

> 

> > The problem there is that the SAW register is also used during

> > idle/suspend when software isn't running. The CPU power down

> > sequence usually turns off the regulator, but it may also change

> > the voltage to something lower, depending on how deep of

> > idle/suspend we're trying to achieve. This is all done without

> > software intervention. When the CPU wakes up due to some

> > interrupt, the SPM runs the CPU power on sequence which takes

> > whatever is in the SAW register and sends it off to the PMIC to

> > restore the CPU voltage. Again, this is all hardware doing this.

> 

> This might be a problem. I guess we can't change this hardware

> behaviour?


Right, we can't change how this hardware works. The way this SAW
regulator driver is written works for this problem though. It
modifies the SAW registers to set the voltage on the CPU that is
using the regulator, thereby preventing the CPU from going idle
or hitting suspend when the voltage is changed. If we were to use
the SSBI/SPMI regulator driver we would need to do something
similar so that the SPM is guaranteed to not be running during
the voltage switch. So I guess schedule a work on the CPU that's
affected by the voltage switch and hope that the CPU doesn't go
offline during that time?

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

Patch

diff --git a/Documentation/devicetree/bindings/regulator/qcom,saw-regulator.txt b/Documentation/devicetree/bindings/regulator/qcom,saw-regulator.txt
new file mode 100644
index 000000000000..977fec08b2ae
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/qcom,saw-regulator.txt
@@ -0,0 +1,31 @@ 
+Qualcomm SAW Regulators
+
+SAW (Subsystem Power Manager and Adaptive Voltage Scaling Wrapper) is a hardware
+block in the Qualcomm chipsets that regulates the power to the CPU cores on devices
+such as APQ8064, MSM8974, APQ8084 and others.
+
+- compatible:
+	Usage: required
+	Value type: <string>
+	Definition: must be one of:
+			"qcom,apq8064-saw2-v1.1-regulator"
+
+Example:
+                saw0: power-controller@2089000 {
+			compatible = "qcom,apq8064-saw2-v1.1-cpu", "qcom,saw2", "syscon", "simple-mfd";
+			reg = <0x02089000 0x1000>, <0x02009000 0x1000>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			saw0_regulator: regulator@2089000 {
+				compatible = "qcom,apq8064-saw2-v1.1-regulator";
+				regulator-always-on;
+				regulator-min-microvolt = <825000>;
+				regulator-max-microvolt = <1250000>;
+			};
+		};
+
+
+		&CPU0 {
+			cpu-supply = <&saw0_regulator>;
+		};
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 74a6354eaefa..6d5f4fce1d75 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -558,6 +558,18 @@  config REGULATOR_QCOM_RPM
 	  Qualcomm RPM as a module. The module will be named
 	  "qcom_rpm-regulator".
 
+config REGULATOR_QCOM_SAW
+	tristate "Qualcomm SAW regulator driver"
+	depends on (ARCH_QCOM || COMPILE_TEST) && MFD_SYSCON
+	help
+	  If you say yes to this option, support will be included for the
+	  regulators providing power to the CPU cores on devices such as
+	  APQ8064.
+
+	  Say M here if you want to include support for the CPU core voltage
+	  regulators as a module. The module will be named
+	  "qcom_saw-regulator".
+
 config REGULATOR_QCOM_SMD_RPM
 	tristate "Qualcomm SMD based RPM regulator driver"
 	depends on QCOM_SMD_RPM
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 348cfd727350..75a0b4a8f1b2 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -64,6 +64,7 @@  obj-$(CONFIG_REGULATOR_MC13XXX_CORE) +=  mc13xxx-regulator-core.o
 obj-$(CONFIG_REGULATOR_MT6311) += mt6311-regulator.o
 obj-$(CONFIG_REGULATOR_MT6397)	+= mt6397-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_RPM) += qcom_rpm-regulator.o
+obj-$(CONFIG_REGULATOR_QCOM_SAW)+= qcom_saw-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_SMD_RPM) += qcom_smd-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_SPMI) += qcom_spmi-regulator.o
 obj-$(CONFIG_REGULATOR_PALMAS) += palmas-regulator.o
diff --git a/drivers/regulator/qcom_saw-regulator.c b/drivers/regulator/qcom_saw-regulator.c
new file mode 100644
index 000000000000..c800f16adaf0
--- /dev/null
+++ b/drivers/regulator/qcom_saw-regulator.c
@@ -0,0 +1,229 @@ 
+/*
+ * Copyright (c) 2016, Linaro Limited. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
+
+#define	SPM_REG_STS_1			0x10
+#define	SPM_REG_VCTL			0x14
+#define	SPM_REG_PMIC_DATA_0		0x28
+#define	SPM_REG_PMIC_DATA_1		0x2c
+#define	SPM_REG_RST			0x30
+
+struct saw_vreg {
+	struct device		*dev;
+	struct regmap		*regmap;
+	struct regulator_desc	rdesc;
+	struct regulator_dev	*rdev;
+	unsigned int		sel;
+};
+
+struct spm_vlevel_data {
+	struct saw_vreg *vreg;
+	unsigned int sel;
+};
+
+static int saw_regulator_get_voltage_sel(struct regulator_dev *rdev)
+{
+	struct saw_vreg *vreg = rdev_get_drvdata(rdev);
+
+	return vreg->sel;
+}
+
+static void smp_set_vdd(void *data)
+{
+	struct spm_vlevel_data *vdata = (struct spm_vlevel_data *)data;
+	struct saw_vreg *vreg = vdata->vreg;
+	unsigned long new_sel = vdata->sel;
+	u32 val, new_val;
+	u32 vctl, data0, data1;
+	unsigned long timeout;
+
+	if (vreg->sel == new_sel)
+		return;
+
+	regmap_read(vreg->regmap, SPM_REG_VCTL, &vctl);
+	regmap_read(vreg->regmap, SPM_REG_PMIC_DATA_0, &data0);
+	regmap_read(vreg->regmap, SPM_REG_PMIC_DATA_1, &data1);
+
+	/* select the band */
+	val = 0x80 | new_sel;
+
+	vctl &= ~0xff;
+	vctl |= val;
+
+	data0 &= ~0xff;
+	data0 |= val;
+
+	data1 &= ~0x3f;
+	data1 |= val & 0x3f;
+	data1 &= ~0x3f0000;
+	data1 |= ((val & 0x3f) << 16);
+
+	regmap_write(vreg->regmap, SPM_REG_RST, 1);
+	regmap_write(vreg->regmap, SPM_REG_VCTL, vctl);
+	regmap_write(vreg->regmap, SPM_REG_PMIC_DATA_0, data0);
+	regmap_write(vreg->regmap, SPM_REG_PMIC_DATA_1, data1);
+
+	timeout = jiffies + usecs_to_jiffies(100);
+	do {
+		regmap_read(vreg->regmap, SPM_REG_STS_1, &new_val);
+		new_val &= 0xff;
+		if (new_val == val) {
+			vreg->sel = new_sel;
+			return;
+		}
+
+		cpu_relax();
+
+	} while (time_before(jiffies, timeout));
+
+	pr_err("%s: Voltage not changed: %#x\n", __func__, new_val);
+}
+
+static int saw_regulator_set_voltage_sel(struct regulator_dev *rdev,
+					 unsigned selector)
+{
+	struct saw_vreg *vreg = rdev_get_drvdata(rdev);
+	struct spm_vlevel_data data;
+	int cpu = rdev_get_id(rdev);
+
+	data.vreg = vreg;
+	data.sel = selector;
+
+	return smp_call_function_single(cpu, smp_set_vdd, &data, true);
+}
+
+static struct regulator_ops saw_regulator_ops = {
+	.list_voltage = regulator_list_voltage_linear_range,
+	.set_voltage_sel = saw_regulator_set_voltage_sel,
+	.get_voltage_sel = saw_regulator_get_voltage_sel,
+	.set_voltage_time_sel = regulator_set_voltage_time_sel,
+};
+
+static struct regulator_desc saw_regulator = {
+	.owner = THIS_MODULE,
+	.type = REGULATOR_VOLTAGE,
+	.ops  = &saw_regulator_ops,
+	.linear_ranges = (struct regulator_linear_range[]) {
+		REGULATOR_LINEAR_RANGE(700000, 0, 56, 12500),
+	},
+	.n_linear_ranges = 1,
+	.n_voltages = 57,
+	.ramp_delay = 1250,
+};
+
+static struct saw_vreg *saw_get_drv(struct platform_device *pdev,
+				    int *vreg_cpu)
+{
+	struct saw_vreg *vreg = NULL;
+	struct device_node *cpu_node, *saw_node;
+	int cpu;
+	bool found;
+
+	for_each_possible_cpu(cpu) {
+		cpu_node = of_cpu_device_node_get(cpu);
+		if (!cpu_node)
+			continue;
+		saw_node = of_parse_phandle(cpu_node, "qcom,saw", 0);
+		found = (saw_node == pdev->dev.of_node->parent);
+		of_node_put(saw_node);
+		of_node_put(cpu_node);
+		if (found)
+			break;
+	}
+
+	if (found) {
+		vreg = devm_kzalloc(&pdev->dev, sizeof(*vreg), GFP_KERNEL);
+		if (vreg)
+			*vreg_cpu = cpu;
+	}
+
+	return vreg;
+}
+
+static const struct of_device_id qcom_saw_regulator_match[] = {
+	{ .compatible = "qcom,apq8064-saw2-v1.1-regulator" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, qcom_saw_regulator_match);
+
+static int qcom_saw_regulator_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct device_node *saw_np;
+	struct saw_vreg *vreg;
+	struct regulator_config config = { };
+	int ret = 0, cpu = 0;
+	char name[] = "kraitXX";
+
+	vreg = saw_get_drv(pdev, &cpu);
+	if (!vreg)
+		return -EINVAL;
+
+	saw_np = of_get_parent(np);
+	if (!saw_np)
+		return -ENODEV;
+
+	vreg->regmap = syscon_node_to_regmap(saw_np);
+	of_node_put(saw_np);
+	if (IS_ERR(config.regmap))
+		return PTR_ERR(config.regmap);
+
+	snprintf(name, sizeof(name), "krait%d", cpu);
+
+	config.regmap = vreg->regmap;
+	config.dev = &pdev->dev;
+	config.of_node = np;
+	config.driver_data = vreg;
+
+	vreg->rdesc = saw_regulator;
+	vreg->rdesc.id = cpu;
+	vreg->rdesc.name = name;
+	config.init_data = of_get_regulator_init_data(&pdev->dev,
+						      pdev->dev.of_node,
+						      &vreg->rdesc);
+
+	vreg->rdev = devm_regulator_register(&pdev->dev, &vreg->rdesc, &config);
+	if (IS_ERR(vreg->rdev)) {
+		ret = PTR_ERR(vreg->rdev);
+		dev_err(dev, "failed to register SAW regulator: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static struct platform_driver qcom_saw_regulator_driver = {
+	.driver = {
+		.name = "qcom-saw-regulator",
+		.of_match_table = qcom_saw_regulator_match,
+	},
+	.probe = qcom_saw_regulator_probe,
+};
+
+module_platform_driver(qcom_saw_regulator_driver);
+
+MODULE_ALIAS("platform:qcom-saw-regulator");
+MODULE_DESCRIPTION("Qualcomm SAW regulator driver");
+MODULE_AUTHOR("Georgi Djakov <georgi.djakov@linaro.org>");
+MODULE_LICENSE("GPL v2");