All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4] regulator: qcom-saw: Add support for SAW regulators
@ 2016-02-09 13:12 Georgi Djakov
  2016-02-09 18:20 ` Applied "regulator: qcom-saw: Add support for SAW regulators" to the regulator tree Mark Brown
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Georgi Djakov @ 2016-02-09 13:12 UTC (permalink / raw)
  To: broonie
  Cc: lgirdwood, andy.gross, lina.iyer, sboyd, linux-kernel,
	linux-arm-msm, georgi.djakov

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
+
+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");

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

* Applied "regulator: qcom-saw: Add support for SAW regulators" to the regulator tree
  2016-02-09 13:12 [PATCH v4] regulator: qcom-saw: Add support for SAW regulators Georgi Djakov
@ 2016-02-09 18:20 ` Mark Brown
  2016-02-09 22:21 ` [PATCH v4] regulator: qcom-saw: Add support for SAW regulators Lina Iyer
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 18+ messages in thread
From: Mark Brown @ 2016-02-09 18:20 UTC (permalink / raw)
  To: Georgi Djakov, Mark Brown; +Cc: linux-kernel

The patch

   regulator: qcom-saw: Add support for SAW regulators

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 18bba3b503556a5e2e00538cbf2cd014fe4a417c Mon Sep 17 00:00:00 2001
From: Georgi Djakov <georgi.djakov@linaro.org>
Date: Tue, 9 Feb 2016 15:12:29 +0200
Subject: [PATCH] regulator: qcom-saw: Add support for SAW regulators

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>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 .../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 8155e80dd3f8..3a61e0ccced3 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -549,6 +549,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 980b1943fa81..af928cc15c23 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -63,6 +63,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");
-- 
2.7.0.rc3

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

* Re: [PATCH v4] regulator: qcom-saw: Add support for SAW regulators
  2016-02-09 13:12 [PATCH v4] regulator: qcom-saw: Add support for SAW regulators Georgi Djakov
  2016-02-09 18:20 ` Applied "regulator: qcom-saw: Add support for SAW regulators" to the regulator tree Mark Brown
@ 2016-02-09 22:21 ` Lina Iyer
  2016-02-10 10:13   ` Mark Brown
  2016-02-10 12:52   ` Georgi Djakov
  2016-02-19 16:07 ` Mark Brown
  2016-03-15  9:29 ` Applied "regulator: qcom-saw: Add support for SAW regulators" to the regulator tree Mark Brown
  3 siblings, 2 replies; 18+ messages in thread
From: Lina Iyer @ 2016-02-09 22:21 UTC (permalink / raw)
  To: Georgi Djakov
  Cc: broonie, lgirdwood, andy.gross, sboyd, linux-kernel, linux-arm-msm

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");

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

* Re: [PATCH v4] regulator: qcom-saw: Add support for SAW regulators
  2016-02-09 22:21 ` [PATCH v4] regulator: qcom-saw: Add support for SAW regulators Lina Iyer
@ 2016-02-10 10:13   ` Mark Brown
  2016-02-10 16:42     ` Lina Iyer
  2016-02-10 12:52   ` Georgi Djakov
  1 sibling, 1 reply; 18+ messages in thread
From: Mark Brown @ 2016-02-10 10:13 UTC (permalink / raw)
  To: Lina Iyer
  Cc: Georgi Djakov, lgirdwood, andy.gross, sboyd, linux-kernel, linux-arm-msm

[-- Attachment #1: Type: text/plain, Size: 1043 bytes --]

On Tue, Feb 09, 2016 at 03:21:54PM -0700, Lina Iyer wrote:
> On Tue, Feb 09 2016 at 06:13 -0700, Georgi Djakov wrote:

Please delete unneeded context from mails when replying.  Doing this
makes it much easier to find your reply in the message, helping ensure
it won't be missed by people scrolling through the irrelevant quoted
material.

> >+#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.

This has already been applied.  It's not immediately clear to me that
changes simply to move the registers around would be more extensive than
just adding fields to the driver data, is it more than just that?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [PATCH v4] regulator: qcom-saw: Add support for SAW regulators
  2016-02-09 22:21 ` [PATCH v4] regulator: qcom-saw: Add support for SAW regulators Lina Iyer
  2016-02-10 10:13   ` Mark Brown
@ 2016-02-10 12:52   ` Georgi Djakov
  2016-02-10 18:36     ` Stephen Boyd
  1 sibling, 1 reply; 18+ messages in thread
From: Georgi Djakov @ 2016-02-10 12:52 UTC (permalink / raw)
  To: Lina Iyer
  Cc: broonie, lgirdwood, andy.gross, sboyd, linux-kernel, linux-arm-msm

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?

[..]
>> +
>> +module_platform_driver(qcom_saw_regulator_driver);
>> +
> builtin_platform_driver() perhaps ?
> 

It's tested as module too, so there is no reason to change to builtin.

Thanks,
Georgi

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

* Re: [PATCH v4] regulator: qcom-saw: Add support for SAW regulators
  2016-02-10 10:13   ` Mark Brown
@ 2016-02-10 16:42     ` Lina Iyer
  2016-02-10 18:54       ` Mark Brown
  0 siblings, 1 reply; 18+ messages in thread
From: Lina Iyer @ 2016-02-10 16:42 UTC (permalink / raw)
  To: Mark Brown
  Cc: Georgi Djakov, lgirdwood, andy.gross, sboyd, linux-kernel, linux-arm-msm

On Wed, Feb 10 2016 at 03:13 -0700, Mark Brown wrote:
>On Tue, Feb 09, 2016 at 03:21:54PM -0700, Lina Iyer wrote:
>> On Tue, Feb 09 2016 at 06:13 -0700, Georgi Djakov wrote:
>
>Please delete unneeded context from mails when replying.  Doing this
>makes it much easier to find your reply in the message, helping ensure
>it won't be missed by people scrolling through the irrelevant quoted
>material.
>
Sorry.

>> >+#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.
>
>This has already been applied.  It's not immediately clear to me that
>changes simply to move the registers around would be more extensive than
>just adding fields to the driver data, is it more than just that?
>
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.

While the regulator changes are orthogonal between 8064 and the rest of
the QCOM SoC's, it might have been nicer to look ahead into other SoC's
that may share similar compatible (with a different version ofcourse),
for the design of this driver. That said, the functionality of the
driver seems like what is needed for 8064 and I wouldnt want to stop the
upstreaming process. It might need changes to abstract offsets,
identifying the SAW regulator that will change the voltage on that SoC
etc., in the future to support both class of SoCs.

Thanks,
Lina

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

* Re: [PATCH v4] regulator: qcom-saw: Add support for SAW regulators
  2016-02-10 12:52   ` Georgi Djakov
@ 2016-02-10 18:36     ` Stephen Boyd
  2016-02-10 18:43       ` Mark Brown
  0 siblings, 1 reply; 18+ messages in thread
From: Stephen Boyd @ 2016-02-10 18:36 UTC (permalink / raw)
  To: Georgi Djakov
  Cc: Lina Iyer, broonie, lgirdwood, andy.gross, linux-kernel, linux-arm-msm

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

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

* Re: [PATCH v4] regulator: qcom-saw: Add support for SAW regulators
  2016-02-10 18:36     ` Stephen Boyd
@ 2016-02-10 18:43       ` Mark Brown
  2016-02-10 19:04         ` Stephen Boyd
  0 siblings, 1 reply; 18+ messages in thread
From: Mark Brown @ 2016-02-10 18:43 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Georgi Djakov, Lina Iyer, lgirdwood, andy.gross, linux-kernel,
	linux-arm-msm

[-- Attachment #1: Type: text/plain, Size: 904 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [PATCH v4] regulator: qcom-saw: Add support for SAW regulators
  2016-02-10 16:42     ` Lina Iyer
@ 2016-02-10 18:54       ` Mark Brown
  0 siblings, 0 replies; 18+ messages in thread
From: Mark Brown @ 2016-02-10 18:54 UTC (permalink / raw)
  To: Lina Iyer
  Cc: Georgi Djakov, lgirdwood, andy.gross, sboyd, linux-kernel, linux-arm-msm

[-- Attachment #1: Type: text/plain, Size: 606 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [PATCH v4] regulator: qcom-saw: Add support for SAW regulators
  2016-02-10 18:43       ` Mark Brown
@ 2016-02-10 19:04         ` Stephen Boyd
  2016-02-10 19:21           ` Mark Brown
  0 siblings, 1 reply; 18+ messages in thread
From: Stephen Boyd @ 2016-02-10 19:04 UTC (permalink / raw)
  To: Mark Brown
  Cc: Georgi Djakov, Lina Iyer, lgirdwood, andy.gross, linux-kernel,
	linux-arm-msm

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

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

* Re: [PATCH v4] regulator: qcom-saw: Add support for SAW regulators
  2016-02-10 19:04         ` Stephen Boyd
@ 2016-02-10 19:21           ` Mark Brown
  2016-02-10 22:46             ` Stephen Boyd
  0 siblings, 1 reply; 18+ messages in thread
From: Mark Brown @ 2016-02-10 19:21 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Georgi Djakov, Lina Iyer, lgirdwood, andy.gross, linux-kernel,
	linux-arm-msm

[-- Attachment #1: Type: text/plain, Size: 1299 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [PATCH v4] regulator: qcom-saw: Add support for SAW regulators
  2016-02-10 19:21           ` Mark Brown
@ 2016-02-10 22:46             ` Stephen Boyd
  2016-02-11 10:17               ` Georgi Djakov
  0 siblings, 1 reply; 18+ messages in thread
From: Stephen Boyd @ 2016-02-10 22:46 UTC (permalink / raw)
  To: Mark Brown
  Cc: Georgi Djakov, Lina Iyer, lgirdwood, andy.gross, linux-kernel,
	linux-arm-msm

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

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

* Re: [PATCH v4] regulator: qcom-saw: Add support for SAW regulators
  2016-02-10 22:46             ` Stephen Boyd
@ 2016-02-11 10:17               ` Georgi Djakov
  2016-02-12  0:17                 ` Stephen Boyd
  0 siblings, 1 reply; 18+ messages in thread
From: Georgi Djakov @ 2016-02-11 10:17 UTC (permalink / raw)
  To: Stephen Boyd, Mark Brown
  Cc: Lina Iyer, lgirdwood, andy.gross, linux-kernel, linux-arm-msm

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?

> 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?

Anyway, thanks for the detailed feedback. This is very helpful,
as I have very limited information on this hardware.

BR,
Georgi

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

* Re: [PATCH v4] regulator: qcom-saw: Add support for SAW regulators
  2016-02-11 10:17               ` Georgi Djakov
@ 2016-02-12  0:17                 ` Stephen Boyd
  2016-02-12 23:03                   ` Mark Brown
  0 siblings, 1 reply; 18+ messages in thread
From: Stephen Boyd @ 2016-02-12  0:17 UTC (permalink / raw)
  To: Georgi Djakov
  Cc: Mark Brown, Lina Iyer, lgirdwood, andy.gross, linux-kernel,
	linux-arm-msm

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

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

* Re: [PATCH v4] regulator: qcom-saw: Add support for SAW regulators
  2016-02-12  0:17                 ` Stephen Boyd
@ 2016-02-12 23:03                   ` Mark Brown
  0 siblings, 0 replies; 18+ messages in thread
From: Mark Brown @ 2016-02-12 23:03 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Georgi Djakov, Lina Iyer, lgirdwood, andy.gross, linux-kernel,
	linux-arm-msm

[-- Attachment #1: Type: text/plain, Size: 1679 bytes --]

On Thu, Feb 11, 2016 at 04:17:55PM -0800, Stephen Boyd wrote:
> On 02/11, Georgi Djakov wrote:

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

So this is sounding like we want to drop this driver?

> 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?

"Hope" doesn't sound like this is going to be a safe long term solution,
either something more integrated with the offline code that explicitly
blocks offlining (which I don't off the top of my head know if we can do
easily) or something where we start something on the CPU and then
explicitly handshake with it (but then what if the CPU has real work to
do?) seems better.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [PATCH v4] regulator: qcom-saw: Add support for SAW regulators
  2016-02-09 13:12 [PATCH v4] regulator: qcom-saw: Add support for SAW regulators Georgi Djakov
  2016-02-09 18:20 ` Applied "regulator: qcom-saw: Add support for SAW regulators" to the regulator tree Mark Brown
  2016-02-09 22:21 ` [PATCH v4] regulator: qcom-saw: Add support for SAW regulators Lina Iyer
@ 2016-02-19 16:07 ` Mark Brown
  2016-03-15  9:29 ` Applied "regulator: qcom-saw: Add support for SAW regulators" to the regulator tree Mark Brown
  3 siblings, 0 replies; 18+ messages in thread
From: Mark Brown @ 2016-02-19 16:07 UTC (permalink / raw)
  To: Georgi Djakov
  Cc: lgirdwood, andy.gross, lina.iyer, sboyd, linux-kernel, linux-arm-msm

[-- Attachment #1: Type: text/plain, Size: 744 bytes --]

On Tue, Feb 09, 2016 at 03:12:29PM +0200, 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.

Following on from the discussion we had here and a bit off offline
discussion with Stephen it seems that we need to drop this since having
it in the DT as a separate regulator doesn't really represent the
hardware.  The SAW is controlling the underlying PMIC regulator and we
will eventually need to control that regulator directly so the SAW
should be presented as an alternative control interface for the PMIC
regulator.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

* Applied "regulator: qcom-saw: Add support for SAW regulators" to the regulator tree
  2016-02-09 13:12 [PATCH v4] regulator: qcom-saw: Add support for SAW regulators Georgi Djakov
                   ` (2 preceding siblings ...)
  2016-02-19 16:07 ` Mark Brown
@ 2016-03-15  9:29 ` Mark Brown
  2016-03-15  9:30   ` Mark Brown
  3 siblings, 1 reply; 18+ messages in thread
From: Mark Brown @ 2016-03-15  9:29 UTC (permalink / raw)
  To: Georgi Djakov, Mark Brown; +Cc: linux-kernel

The patch

   regulator: qcom-saw: Add support for SAW regulators

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 18bba3b503556a5e2e00538cbf2cd014fe4a417c Mon Sep 17 00:00:00 2001
From: Georgi Djakov <georgi.djakov@linaro.org>
Date: Tue, 9 Feb 2016 15:12:29 +0200
Subject: [PATCH] regulator: qcom-saw: Add support for SAW regulators

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>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 .../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 8155e80dd3f8..3a61e0ccced3 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -549,6 +549,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 980b1943fa81..af928cc15c23 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -63,6 +63,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");
-- 
2.7.0

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

* Re: Applied "regulator: qcom-saw: Add support for SAW regulators" to the regulator tree
  2016-03-15  9:29 ` Applied "regulator: qcom-saw: Add support for SAW regulators" to the regulator tree Mark Brown
@ 2016-03-15  9:30   ` Mark Brown
  0 siblings, 0 replies; 18+ messages in thread
From: Mark Brown @ 2016-03-15  9:30 UTC (permalink / raw)
  To: Georgi Djakov; +Cc: linux-kernel

[-- Attachment #1: Type: text/plain, Size: 339 bytes --]

On Tue, Mar 15, 2016 at 09:29:12AM +0000, Mark Brown wrote:
> The patch
> 
>    regulator: qcom-saw: Add support for SAW regulators
> 
> has been applied to the regulator tree at
> 
>    git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

Sorry, this is my scripts not coping with completely removing a branch.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

end of thread, other threads:[~2016-03-15  9:31 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-09 13:12 [PATCH v4] regulator: qcom-saw: Add support for SAW regulators Georgi Djakov
2016-02-09 18:20 ` Applied "regulator: qcom-saw: Add support for SAW regulators" to the regulator tree Mark Brown
2016-02-09 22:21 ` [PATCH v4] regulator: qcom-saw: Add support for SAW regulators Lina Iyer
2016-02-10 10:13   ` Mark Brown
2016-02-10 16:42     ` Lina Iyer
2016-02-10 18:54       ` Mark Brown
2016-02-10 12:52   ` Georgi Djakov
2016-02-10 18:36     ` Stephen Boyd
2016-02-10 18:43       ` Mark Brown
2016-02-10 19:04         ` Stephen Boyd
2016-02-10 19:21           ` Mark Brown
2016-02-10 22:46             ` Stephen Boyd
2016-02-11 10:17               ` Georgi Djakov
2016-02-12  0:17                 ` Stephen Boyd
2016-02-12 23:03                   ` Mark Brown
2016-02-19 16:07 ` Mark Brown
2016-03-15  9:29 ` Applied "regulator: qcom-saw: Add support for SAW regulators" to the regulator tree Mark Brown
2016-03-15  9:30   ` Mark Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.