[PATCH 0/3] regulator: s2mps11: Add support for S2MPS14 regulators

[PATCH 0/3] regulator: s2mps11: Add support for S2MPS14 regulators

[Krzysztof Kozlowski]

Hi,

This patchset adds support for the S2MPS14 device to the s2mps11 regulator
driver. It's a subset of previously sent patches for S2MPS14 device:
http://thread.gmane.org/gmane.linux.kernel.samsung-soc/27194/focus=1649217
along with one new patch:
 - PATCH 2/3: regulator: s2mps11: Add set_suspend_disable for S2MPS14
which replaces previous "opmode" idea.


These patches are rebased against linux-next tree (next-20140305) because
they depend on changes in main MFD sec-core and s2mps11 regulator driver.


Best regards,
Krzysztof

Krzysztof Kozlowski (3):
  regulator: s2mps11: Add support for S2MPS14 regulators
  regulator: s2mps11: Add set_suspend_disable for S2MPS14
  Documentation: mfd: s2mps11: Document support for S2MPS14

 Documentation/devicetree/bindings/mfd/s2mps11.txt |   12 +-
 drivers/regulator/Kconfig                         |    9 +-
 drivers/regulator/s2mps11.c                       |  278 ++++++++++++++++-----
 include/linux/mfd/samsung/s2mps14.h               |    2 +
 4 files changed, 232 insertions(+), 69 deletions(-)

-- 
1.7.9.5

[PATCH 1/3] regulator: s2mps11: Add support for S2MPS14 regulators

[Krzysztof Kozlowski]

Add support for S2MPS14 PMIC regulators to s2mps11 driver. The S2MPS14
has fewer BUCK-s and LDO-s than S2MPS11. It also does not support
controlling the BUCK ramp delay.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Reviewed-by: Yadwinder Singh Brar <yadi.brar@samsung.com>
---
 drivers/regulator/Kconfig   |    9 +-
 drivers/regulator/s2mps11.c |  252 ++++++++++++++++++++++++++++++++-----------
 2 files changed, 196 insertions(+), 65 deletions(-)

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 4ddfb6c065c7..14ff714a25fb 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -426,12 +426,13 @@ config REGULATOR_RC5T583
 	  outputs which can be controlled by i2c communication.
 
 config REGULATOR_S2MPS11
-	tristate "Samsung S2MPS11 voltage regulator"
+	tristate "Samsung S2MPS11/S2MPS14 voltage regulator"
 	depends on MFD_SEC_CORE
 	help
-	 This driver supports a Samsung S2MPS11 voltage output regulator
-	 via I2C bus. S2MPS11 is comprised of high efficient Buck converters
-	 including Dual-Phase Buck converter, Buck-Boost converter, various LDOs.
+	 This driver supports a Samsung S2MPS11/S2MPS14 voltage output
+	 regulator via I2C bus. The chip is comprised of high efficient Buck
+	 converters including Dual-Phase Buck converter, Buck-Boost converter,
+	 various LDOs.
 
 config REGULATOR_S5M8767
 	tristate "Samsung S5M8767A voltage regulator"
diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
index b51ccf534a7c..1a732e54cd70 100644
--- a/drivers/regulator/s2mps11.c
+++ b/drivers/regulator/s2mps11.c
@@ -1,13 +1,18 @@
 /*
  * s2mps11.c
  *
- * Copyright (c) 2012 Samsung Electronics Co., Ltd
+ * Copyright (c) 2012-2014 Samsung Electronics Co., Ltd
  *              http://www.samsung.com
  *
- *  This program is free software; you can redistribute  it and/or modify it
- *  under  the terms of  the GNU General  Public License as published by the
- *  Free Software Foundation;  either version 2 of the  License, or (at your
- *  option) any later version.
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ *
+ * 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.
  *
  */
 
@@ -24,6 +29,7 @@
 #include <linux/regulator/of_regulator.h>
 #include <linux/mfd/samsung/core.h>
 #include <linux/mfd/samsung/s2mps11.h>
+#include <linux/mfd/samsung/s2mps14.h>
 
 struct s2mps11_info {
 	unsigned int rdev_num;
@@ -233,7 +239,7 @@ static struct regulator_ops s2mps11_buck_ops = {
 	.set_ramp_delay		= s2mps11_set_ramp_delay,
 };
 
-#define regulator_desc_ldo1(num)	{		\
+#define regulator_desc_s2mps11_ldo1(num)	{		\
 	.name		= "LDO"#num,			\
 	.id		= S2MPS11_LDO##num,		\
 	.ops		= &s2mps11_ldo_ops,		\
@@ -247,7 +253,7 @@ static struct regulator_ops s2mps11_buck_ops = {
 	.enable_reg	= S2MPS11_REG_L1CTRL + num - 1,	\
 	.enable_mask	= S2MPS11_ENABLE_MASK		\
 }
-#define regulator_desc_ldo2(num)	{		\
+#define regulator_desc_s2mps11_ldo2(num) {		\
 	.name		= "LDO"#num,			\
 	.id		= S2MPS11_LDO##num,		\
 	.ops		= &s2mps11_ldo_ops,		\
@@ -262,7 +268,7 @@ static struct regulator_ops s2mps11_buck_ops = {
 	.enable_mask	= S2MPS11_ENABLE_MASK		\
 }
 
-#define regulator_desc_buck1_4(num)	{			\
+#define regulator_desc_s2mps11_buck1_4(num) {			\
 	.name		= "BUCK"#num,				\
 	.id		= S2MPS11_BUCK##num,			\
 	.ops		= &s2mps11_buck_ops,			\
@@ -278,7 +284,7 @@ static struct regulator_ops s2mps11_buck_ops = {
 	.enable_mask	= S2MPS11_ENABLE_MASK			\
 }
 
-#define regulator_desc_buck5	{				\
+#define regulator_desc_s2mps11_buck5 {				\
 	.name		= "BUCK5",				\
 	.id		= S2MPS11_BUCK5,			\
 	.ops		= &s2mps11_buck_ops,			\
@@ -294,7 +300,7 @@ static struct regulator_ops s2mps11_buck_ops = {
 	.enable_mask	= S2MPS11_ENABLE_MASK			\
 }
 
-#define regulator_desc_buck6_8(num)	{			\
+#define regulator_desc_s2mps11_buck6_8(num) {			\
 	.name		= "BUCK"#num,				\
 	.id		= S2MPS11_BUCK##num,			\
 	.ops		= &s2mps11_buck_ops,			\
@@ -310,7 +316,7 @@ static struct regulator_ops s2mps11_buck_ops = {
 	.enable_mask	= S2MPS11_ENABLE_MASK			\
 }
 
-#define regulator_desc_buck9	{				\
+#define regulator_desc_s2mps11_buck9 {				\
 	.name		= "BUCK9",				\
 	.id		= S2MPS11_BUCK9,			\
 	.ops		= &s2mps11_buck_ops,			\
@@ -326,7 +332,7 @@ static struct regulator_ops s2mps11_buck_ops = {
 	.enable_mask	= S2MPS11_ENABLE_MASK			\
 }
 
-#define regulator_desc_buck10	{				\
+#define regulator_desc_s2mps11_buck10 {				\
 	.name		= "BUCK10",				\
 	.id		= S2MPS11_BUCK10,			\
 	.ops		= &s2mps11_buck_ops,			\
@@ -343,54 +349,173 @@ static struct regulator_ops s2mps11_buck_ops = {
 }
 
 static const struct regulator_desc s2mps11_regulators[] = {
-	regulator_desc_ldo2(1),
-	regulator_desc_ldo1(2),
-	regulator_desc_ldo1(3),
-	regulator_desc_ldo1(4),
-	regulator_desc_ldo1(5),
-	regulator_desc_ldo2(6),
-	regulator_desc_ldo1(7),
-	regulator_desc_ldo1(8),
-	regulator_desc_ldo1(9),
-	regulator_desc_ldo1(10),
-	regulator_desc_ldo2(11),
-	regulator_desc_ldo1(12),
-	regulator_desc_ldo1(13),
-	regulator_desc_ldo1(14),
-	regulator_desc_ldo1(15),
-	regulator_desc_ldo1(16),
-	regulator_desc_ldo1(17),
-	regulator_desc_ldo1(18),
-	regulator_desc_ldo1(19),
-	regulator_desc_ldo1(20),
-	regulator_desc_ldo1(21),
-	regulator_desc_ldo2(22),
-	regulator_desc_ldo2(23),
-	regulator_desc_ldo1(24),
-	regulator_desc_ldo1(25),
-	regulator_desc_ldo1(26),
-	regulator_desc_ldo2(27),
-	regulator_desc_ldo1(28),
-	regulator_desc_ldo1(29),
-	regulator_desc_ldo1(30),
-	regulator_desc_ldo1(31),
-	regulator_desc_ldo1(32),
-	regulator_desc_ldo1(33),
-	regulator_desc_ldo1(34),
-	regulator_desc_ldo1(35),
-	regulator_desc_ldo1(36),
-	regulator_desc_ldo1(37),
-	regulator_desc_ldo1(38),
-	regulator_desc_buck1_4(1),
-	regulator_desc_buck1_4(2),
-	regulator_desc_buck1_4(3),
-	regulator_desc_buck1_4(4),
-	regulator_desc_buck5,
-	regulator_desc_buck6_8(6),
-	regulator_desc_buck6_8(7),
-	regulator_desc_buck6_8(8),
-	regulator_desc_buck9,
-	regulator_desc_buck10,
+	regulator_desc_s2mps11_ldo2(1),
+	regulator_desc_s2mps11_ldo1(2),
+	regulator_desc_s2mps11_ldo1(3),
+	regulator_desc_s2mps11_ldo1(4),
+	regulator_desc_s2mps11_ldo1(5),
+	regulator_desc_s2mps11_ldo2(6),
+	regulator_desc_s2mps11_ldo1(7),
+	regulator_desc_s2mps11_ldo1(8),
+	regulator_desc_s2mps11_ldo1(9),
+	regulator_desc_s2mps11_ldo1(10),
+	regulator_desc_s2mps11_ldo2(11),
+	regulator_desc_s2mps11_ldo1(12),
+	regulator_desc_s2mps11_ldo1(13),
+	regulator_desc_s2mps11_ldo1(14),
+	regulator_desc_s2mps11_ldo1(15),
+	regulator_desc_s2mps11_ldo1(16),
+	regulator_desc_s2mps11_ldo1(17),
+	regulator_desc_s2mps11_ldo1(18),
+	regulator_desc_s2mps11_ldo1(19),
+	regulator_desc_s2mps11_ldo1(20),
+	regulator_desc_s2mps11_ldo1(21),
+	regulator_desc_s2mps11_ldo2(22),
+	regulator_desc_s2mps11_ldo2(23),
+	regulator_desc_s2mps11_ldo1(24),
+	regulator_desc_s2mps11_ldo1(25),
+	regulator_desc_s2mps11_ldo1(26),
+	regulator_desc_s2mps11_ldo2(27),
+	regulator_desc_s2mps11_ldo1(28),
+	regulator_desc_s2mps11_ldo1(29),
+	regulator_desc_s2mps11_ldo1(30),
+	regulator_desc_s2mps11_ldo1(31),
+	regulator_desc_s2mps11_ldo1(32),
+	regulator_desc_s2mps11_ldo1(33),
+	regulator_desc_s2mps11_ldo1(34),
+	regulator_desc_s2mps11_ldo1(35),
+	regulator_desc_s2mps11_ldo1(36),
+	regulator_desc_s2mps11_ldo1(37),
+	regulator_desc_s2mps11_ldo1(38),
+	regulator_desc_s2mps11_buck1_4(1),
+	regulator_desc_s2mps11_buck1_4(2),
+	regulator_desc_s2mps11_buck1_4(3),
+	regulator_desc_s2mps11_buck1_4(4),
+	regulator_desc_s2mps11_buck5,
+	regulator_desc_s2mps11_buck6_8(6),
+	regulator_desc_s2mps11_buck6_8(7),
+	regulator_desc_s2mps11_buck6_8(8),
+	regulator_desc_s2mps11_buck9,
+	regulator_desc_s2mps11_buck10,
+};
+
+static struct regulator_ops s2mps14_reg_ops = {
+	.list_voltage		= regulator_list_voltage_linear,
+	.map_voltage		= regulator_map_voltage_linear,
+	.is_enabled		= regulator_is_enabled_regmap,
+	.enable			= regulator_enable_regmap,
+	.disable		= regulator_disable_regmap,
+	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
+	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
+	.set_voltage_time_sel	= regulator_set_voltage_time_sel,
+};
+
+#define regulator_desc_s2mps14_ldo1(num) {		\
+	.name		= "LDO"#num,			\
+	.id		= S2MPS14_LDO##num,		\
+	.ops		= &s2mps14_reg_ops,		\
+	.type		= REGULATOR_VOLTAGE,		\
+	.owner		= THIS_MODULE,			\
+	.min_uV		= S2MPS14_LDO_MIN_800MV,	\
+	.uV_step	= S2MPS14_LDO_STEP_25MV,	\
+	.n_voltages	= S2MPS14_LDO_N_VOLTAGES,	\
+	.vsel_reg	= S2MPS14_REG_L1CTRL + num - 1,	\
+	.vsel_mask	= S2MPS14_LDO_VSEL_MASK,	\
+	.enable_reg	= S2MPS14_REG_L1CTRL + num - 1,	\
+	.enable_mask	= S2MPS14_ENABLE_MASK		\
+}
+#define regulator_desc_s2mps14_ldo2(num) {		\
+	.name		= "LDO"#num,			\
+	.id		= S2MPS14_LDO##num,		\
+	.ops		= &s2mps14_reg_ops,		\
+	.type		= REGULATOR_VOLTAGE,		\
+	.owner		= THIS_MODULE,			\
+	.min_uV		= S2MPS14_LDO_MIN_1800MV,	\
+	.uV_step	= S2MPS14_LDO_STEP_25MV,	\
+	.n_voltages	= S2MPS14_LDO_N_VOLTAGES,	\
+	.vsel_reg	= S2MPS14_REG_L1CTRL + num - 1,	\
+	.vsel_mask	= S2MPS14_LDO_VSEL_MASK,	\
+	.enable_reg	= S2MPS14_REG_L1CTRL + num - 1,	\
+	.enable_mask	= S2MPS14_ENABLE_MASK		\
+}
+#define regulator_desc_s2mps14_ldo3(num) {		\
+	.name		= "LDO"#num,			\
+	.id		= S2MPS14_LDO##num,		\
+	.ops		= &s2mps14_reg_ops,		\
+	.type		= REGULATOR_VOLTAGE,		\
+	.owner		= THIS_MODULE,			\
+	.min_uV		= S2MPS14_LDO_MIN_800MV,	\
+	.uV_step	= S2MPS14_LDO_STEP_12_5MV,	\
+	.n_voltages	= S2MPS14_LDO_N_VOLTAGES,	\
+	.vsel_reg	= S2MPS14_REG_L1CTRL + num - 1,	\
+	.vsel_mask	= S2MPS14_LDO_VSEL_MASK,	\
+	.enable_reg	= S2MPS14_REG_L1CTRL + num - 1,	\
+	.enable_mask	= S2MPS14_ENABLE_MASK		\
+}
+#define regulator_desc_s2mps14_buck1235(num) {			\
+	.name		= "BUCK"#num,				\
+	.id		= S2MPS14_BUCK##num,			\
+	.ops		= &s2mps14_reg_ops,			\
+	.type		= REGULATOR_VOLTAGE,			\
+	.owner		= THIS_MODULE,				\
+	.min_uV		= S2MPS14_BUCK1235_MIN_600MV,		\
+	.uV_step	= S2MPS14_BUCK1235_STEP_6_25MV,		\
+	.n_voltages	= S2MPS14_BUCK_N_VOLTAGES,		\
+	.linear_min_sel = S2MPS14_BUCK1235_START_SEL,		\
+	.ramp_delay	= S2MPS14_BUCK_RAMP_DELAY,		\
+	.vsel_reg	= S2MPS14_REG_B1CTRL2 + (num - 1) * 2,	\
+	.vsel_mask	= S2MPS14_BUCK_VSEL_MASK,		\
+	.enable_reg	= S2MPS14_REG_B1CTRL1 + (num - 1) * 2,	\
+	.enable_mask	= S2MPS14_ENABLE_MASK			\
+}
+#define regulator_desc_s2mps14_buck4(num) {			\
+	.name		= "BUCK"#num,				\
+	.id		= S2MPS14_BUCK##num,			\
+	.ops		= &s2mps14_reg_ops,			\
+	.type		= REGULATOR_VOLTAGE,			\
+	.owner		= THIS_MODULE,				\
+	.min_uV		= S2MPS14_BUCK4_MIN_1400MV,		\
+	.uV_step	= S2MPS14_BUCK4_STEP_12_5MV,		\
+	.n_voltages	= S2MPS14_BUCK_N_VOLTAGES,		\
+	.linear_min_sel = S2MPS14_BUCK4_START_SEL,		\
+	.ramp_delay	= S2MPS14_BUCK_RAMP_DELAY,		\
+	.vsel_reg	= S2MPS14_REG_B1CTRL2 + (num - 1) * 2,	\
+	.vsel_mask	= S2MPS14_BUCK_VSEL_MASK,		\
+	.enable_reg	= S2MPS14_REG_B1CTRL1 + (num - 1) * 2,	\
+	.enable_mask	= S2MPS14_ENABLE_MASK			\
+}
+
+static const struct regulator_desc s2mps14_regulators[] = {
+	regulator_desc_s2mps14_ldo3(1),
+	regulator_desc_s2mps14_ldo3(2),
+	regulator_desc_s2mps14_ldo1(3),
+	regulator_desc_s2mps14_ldo1(4),
+	regulator_desc_s2mps14_ldo3(5),
+	regulator_desc_s2mps14_ldo3(6),
+	regulator_desc_s2mps14_ldo1(7),
+	regulator_desc_s2mps14_ldo2(8),
+	regulator_desc_s2mps14_ldo3(9),
+	regulator_desc_s2mps14_ldo3(10),
+	regulator_desc_s2mps14_ldo1(11),
+	regulator_desc_s2mps14_ldo2(12),
+	regulator_desc_s2mps14_ldo2(13),
+	regulator_desc_s2mps14_ldo2(14),
+	regulator_desc_s2mps14_ldo2(15),
+	regulator_desc_s2mps14_ldo2(16),
+	regulator_desc_s2mps14_ldo2(17),
+	regulator_desc_s2mps14_ldo2(18),
+	regulator_desc_s2mps14_ldo1(19),
+	regulator_desc_s2mps14_ldo1(20),
+	regulator_desc_s2mps14_ldo1(21),
+	regulator_desc_s2mps14_ldo3(22),
+	regulator_desc_s2mps14_ldo1(23),
+	regulator_desc_s2mps14_ldo2(24),
+	regulator_desc_s2mps14_ldo2(25),
+	regulator_desc_s2mps14_buck1235(1),
+	regulator_desc_s2mps14_buck1235(2),
+	regulator_desc_s2mps14_buck1235(3),
+	regulator_desc_s2mps14_buck4(4),
+	regulator_desc_s2mps14_buck1235(5),
 };
 
 static int s2mps11_pmic_probe(struct platform_device *pdev)
@@ -416,6 +541,10 @@ static int s2mps11_pmic_probe(struct platform_device *pdev)
 		s2mps11->rdev_num = ARRAY_SIZE(s2mps11_regulators);
 		regulators = s2mps11_regulators;
 		break;
+	case S2MPS14X:
+		s2mps11->rdev_num = ARRAY_SIZE(s2mps14_regulators);
+		regulators = s2mps14_regulators;
+		break;
 	default:
 		dev_err(&pdev->dev, "Invalid device type: %u\n", dev_type);
 		return -EINVAL;
@@ -482,6 +611,7 @@ out:
 
 static const struct platform_device_id s2mps11_pmic_id[] = {
 	{ "s2mps11-pmic", S2MPS11X},
+	{ "s2mps14-pmic", S2MPS14X},
 	{ },
 };
 MODULE_DEVICE_TABLE(platform, s2mps11_pmic_id);
@@ -509,5 +639,5 @@ module_exit(s2mps11_pmic_exit);
 
 /* Module information */
 MODULE_AUTHOR("Sangbeom Kim <sbkim73@samsung.com>");
-MODULE_DESCRIPTION("SAMSUNG S2MPS11 Regulator Driver");
+MODULE_DESCRIPTION("SAMSUNG S2MPS11/S2MPS14 Regulator Driver");
 MODULE_LICENSE("GPL");
-- 
1.7.9.5

[PATCH 2/3] regulator: s2mps11: Add set_suspend_disable for S2MPS14

[Krzysztof Kozlowski]

S2MPS14 regulators support suspend mode where their status is controlled
by PWREN coming from SoC. This patch implements the set_suspend_disable
for S2MPS14 regulators.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/regulator/s2mps11.c         |   26 ++++++++++++++++++++++++++
 include/linux/mfd/samsung/s2mps14.h |    2 ++
 2 files changed, 28 insertions(+)

diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
index 1a732e54cd70..d3209acefd79 100644
--- a/drivers/regulator/s2mps11.c
+++ b/drivers/regulator/s2mps11.c
@@ -399,6 +399,31 @@ static const struct regulator_desc s2mps11_regulators[] = {
 	regulator_desc_s2mps11_buck10,
 };
 
+static int s2mps14_set_suspend_disable(struct regulator_dev *rdev)
+{
+	int ret;
+	unsigned int data;
+
+	/* LDO3 should be always on and does not support suspend mode */
+	if (rdev_get_id(rdev) == S2MPS14_LDO3)
+		return 0;
+
+	ret = regmap_read(rdev->regmap, rdev->desc->enable_reg, &data);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * Don't enable suspend mode if regulator is already disabled because
+	 * this would effectively for a short time turn on the regulator after
+	 * resuming.
+	 */
+	if (!(data & rdev->desc->enable_mask))
+		return 0;
+
+	return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
+			rdev->desc->enable_mask, S2MPS14_ENABLE_SUSPEND);
+}
+
 static struct regulator_ops s2mps14_reg_ops = {
 	.list_voltage		= regulator_list_voltage_linear,
 	.map_voltage		= regulator_map_voltage_linear,
@@ -408,6 +433,7 @@ static struct regulator_ops s2mps14_reg_ops = {
 	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
 	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
 	.set_voltage_time_sel	= regulator_set_voltage_time_sel,
+	.set_suspend_disable	= s2mps14_set_suspend_disable,
 };
 
 #define regulator_desc_s2mps14_ldo1(num) {		\
diff --git a/include/linux/mfd/samsung/s2mps14.h b/include/linux/mfd/samsung/s2mps14.h
index ec1e0857ddde..4b449b8ac548 100644
--- a/include/linux/mfd/samsung/s2mps14.h
+++ b/include/linux/mfd/samsung/s2mps14.h
@@ -146,6 +146,8 @@ enum s2mps14_regulators {
 #define S2MPS14_BUCK_VSEL_MASK		0xFF
 #define S2MPS14_ENABLE_MASK		(0x03 << S2MPS14_ENABLE_SHIFT)
 #define S2MPS14_ENABLE_SHIFT		6
+/* On/Off controlled by PWREN */
+#define S2MPS14_ENABLE_SUSPEND		(0x01 << S2MPS14_ENABLE_SHIFT)
 #define S2MPS14_LDO_N_VOLTAGES		(S2MPS14_LDO_VSEL_MASK + 1)
 #define S2MPS14_BUCK_N_VOLTAGES		(S2MPS14_BUCK_VSEL_MASK + 1)
 
-- 
1.7.9.5

Re: [PATCH 2/3] regulator: s2mps11: Add set_suspend_disable for S2MPS14

[Lee Jones]

> S2MPS14 regulators support suspend mode where their status is controlled
> by PWREN coming from SoC. This patch implements the set_suspend_disable
> for S2MPS14 regulators.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---
>  drivers/regulator/s2mps11.c         |   26 ++++++++++++++++++++++++++
>  include/linux/mfd/samsung/s2mps14.h |    2 ++
>  2 files changed, 28 insertions(+)

For the MFD changes:
  Acked-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 2/3] regulator: s2mps11: Add set_suspend_disable for S2MPS14

[Mark Brown]

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

On Wed, Mar 05, 2014 at 10:22:52AM +0100, Krzysztof Kozlowski wrote:

> +	ret = regmap_read(rdev->regmap, rdev->desc->enable_reg, &data);
> +	if (ret < 0)
> +		return ret;
> +
> +	/*
> +	 * Don't enable suspend mode if regulator is already disabled because
> +	 * this would effectively for a short time turn on the regulator after
> +	 * resuming.
> +	 */
> +	if (!(data & rdev->desc->enable_mask))
> +		return 0;
> +
> +	return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
> +			rdev->desc->enable_mask, S2MPS14_ENABLE_SUSPEND);
> +}

What happens if the regulator gets enabled between this being called
and the device suspending?  I'd expect this to be storing the state and
then changing what gets written during enable and disable operations (if
the hardware does what I think it does).

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/3] regulator: s2mps11: Add set_suspend_disable for S2MPS14

[Krzysztof Kozlowski]

On Thu, 2014-03-06 at 17:38 +0800, Mark Brown wrote:
> On Wed, Mar 05, 2014 at 10:22:52AM +0100, Krzysztof Kozlowski wrote:
> 
> > +	ret = regmap_read(rdev->regmap, rdev->desc->enable_reg, &data);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	/*
> > +	 * Don't enable suspend mode if regulator is already disabled because
> > +	 * this would effectively for a short time turn on the regulator after
> > +	 * resuming.
> > +	 */
> > +	if (!(data & rdev->desc->enable_mask))
> > +		return 0;
> > +
> > +	return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
> > +			rdev->desc->enable_mask, S2MPS14_ENABLE_SUSPEND);
> > +}
> 
> What happens if the regulator gets enabled between this being called
> and the device suspending?  I'd expect this to be storing the state and
> then changing what gets written during enable and disable operations (if
> the hardware does what I think it does).

Hmmm... you're right. I'll fix this.

Anyway can you look at other two patches and apply them if they are OK?
They don't depend on this.

Best regards,
Krzysztof

Re: [PATCH 2/3] regulator: s2mps11: Add set_suspend_disable for S2MPS14

[Mark Brown]

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

On Thu, Mar 06, 2014 at 10:48:54AM +0100, Krzysztof Kozlowski wrote:

> Anyway can you look at other two patches and apply them if they are OK?
> They don't depend on this.

There appears to be some external dependency for the definition of
macros like S2MPS14X?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/3] regulator: s2mps11: Add set_suspend_disable for S2MPS14

[Krzysztof Kozlowski]

On Fri, 2014-03-07 at 09:51 +0800, Mark Brown wrote:
> On Thu, Mar 06, 2014 at 10:48:54AM +0100, Krzysztof Kozlowski wrote:
> 
> > Anyway can you look at other two patches and apply them if they are OK?
> > They don't depend on this.
> 
> There appears to be some external dependency for the definition of
> macros like S2MPS14X?
The patchset depends on S2MPS14 support for MFD driver. This was applied
by Lee here:
https://git.linaro.org/people/lee.jones/mfd.git/shortlog/refs/heads/for-mfd-next
and merged into linux-next.

Best regards,
Krzysztof

Re: [PATCH 2/3] regulator: s2mps11: Add set_suspend_disable for S2MPS14

[Lee Jones]

> > > Anyway can you look at other two patches and apply them if they are OK?
> > > They don't depend on this.
> > 
> > There appears to be some external dependency for the definition of
> > macros like S2MPS14X?
> The patchset depends on S2MPS14 support for MFD driver. This was applied
> by Lee here:
> https://git.linaro.org/people/lee.jones/mfd.git/shortlog/refs/heads/for-mfd-next
> and merged into linux-next.

Right, so when this set obtains all its Acks, I'll apply them to the
MFD branch.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 2/3] regulator: s2mps11: Add set_suspend_disable for S2MPS14

[Krzysztof Kozlowski]

On Thu, 2014-03-06 at 17:38 +0800, Mark Brown wrote:
> On Wed, Mar 05, 2014 at 10:22:52AM +0100, Krzysztof Kozlowski wrote:
> 
> > +	ret = regmap_read(rdev->regmap, rdev->desc->enable_reg, &data);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	/*
> > +	 * Don't enable suspend mode if regulator is already disabled because
> > +	 * this would effectively for a short time turn on the regulator after
> > +	 * resuming.
> > +	 */
> > +	if (!(data & rdev->desc->enable_mask))
> > +		return 0;
> > +
> > +	return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
> > +			rdev->desc->enable_mask, S2MPS14_ENABLE_SUSPEND);
> > +}
> 
> What happens if the regulator gets enabled between this being called
> and the device suspending?  I'd expect this to be storing the state and
> then changing what gets written during enable and disable operations (if
> the hardware does what I think it does).

It seems that none of the regulators implementing set_suspend_disable()
work that way. They just write the suspend value to device. Of course
this is not an issue - the S2MPS14 may be the first :).

However in that case the driver won't be able later to change that value
back to "normal enable" (enable_mask). Consider such flow:
1. System is going to suspend.
2. Some regulator has "rstate->disabled" so set_suspend_disable() is
called on it.
3. The "suspend" value is written to the device for given regulator and
it is stored as "enable" value.
4. If regulator is enabled during here then the same "suspend" value
will be written.
5. System is suspended.
6. After resuming regulator_suspend_finish() calls
_regulator_do_enable() on the regulator... which will write the
"suspend" value because the driver cannot differentiate between this
enable and previous.

I assume that this may not be a problem because:
1. Regulator will be still turned on (the "suspend" value tells PMIC to
enable the regulator when SoC enables power).
2. The first disable of regulator may bring back "enable" value back to
normal mode.

Am I thinking here correctly? 


Best regards,
Krzysztof

Re: [PATCH 2/3] regulator: s2mps11: Add set_suspend_disable for S2MPS14

[Mark Brown]

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

On Thu, Mar 06, 2014 at 03:42:22PM +0100, Krzysztof Kozlowski wrote:

> However in that case the driver won't be able later to change that value
> back to "normal enable" (enable_mask). Consider such flow:
> 1. System is going to suspend.
> 2. Some regulator has "rstate->disabled" so set_suspend_disable() is
> called on it.
> 3. The "suspend" value is written to the device for given regulator and
> it is stored as "enable" value.
> 4. If regulator is enabled during here then the same "suspend" value
> will be written.
> 5. System is suspended.
> 6. After resuming regulator_suspend_finish() calls
> _regulator_do_enable() on the regulator... which will write the
> "suspend" value because the driver cannot differentiate between this
> enable and previous.

> I assume that this may not be a problem because:
> 1. Regulator will be still turned on (the "suspend" value tells PMIC to
> enable the regulator when SoC enables power).
> 2. The first disable of regulator may bring back "enable" value back to
> normal mode.

> Am I thinking here correctly? 

I'm not entirely sure I follow here.  Why would a disable reset the
enable value?  My understanding is that this is a bitfield with several
values, off, on always and on when they system is active.  The suspend
state is being tracked with a variable so I'm not sure why disabling
would reset it?

There is a bit of an issue if the regulator is disabled during runtime
but enabled in suspend but that's hard to resolve and I'm not sure that
it's a realistic issue.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/3] regulator: s2mps11: Add set_suspend_disable for S2MPS14

[Krzysztof Kozlowski]

On Fri, 2014-03-07 at 10:37 +0800, Mark Brown wrote:
> On Thu, Mar 06, 2014 at 03:42:22PM +0100, Krzysztof Kozlowski wrote:
> 
> > However in that case the driver won't be able later to change that value
> > back to "normal enable" (enable_mask). Consider such flow:
> > 1. System is going to suspend.
> > 2. Some regulator has "rstate->disabled" so set_suspend_disable() is
> > called on it.
> > 3. The "suspend" value is written to the device for given regulator and
> > it is stored as "enable" value.
> > 4. If regulator is enabled during here then the same "suspend" value
> > will be written.
> > 5. System is suspended.
> > 6. After resuming regulator_suspend_finish() calls
> > _regulator_do_enable() on the regulator... which will write the
> > "suspend" value because the driver cannot differentiate between this
> > enable and previous.
> 
> > I assume that this may not be a problem because:
> > 1. Regulator will be still turned on (the "suspend" value tells PMIC to
> > enable the regulator when SoC enables power).
> > 2. The first disable of regulator may bring back "enable" value back to
> > normal mode.
> 
> > Am I thinking here correctly? 
> 
> I'm not entirely sure I follow here.  Why would a disable reset the
> enable value?  My understanding is that this is a bitfield with several
> values, off, on always and on when they system is active.  The suspend
> state is being tracked with a variable so I'm not sure why disabling
> would reset it?
> 
> There is a bit of an issue if the regulator is disabled during runtime
> but enabled in suspend but that's hard to resolve and I'm not sure that
> it's a realistic issue.

OK, I think I understand it... I sent v2 of the set_suspend_disable
patch.

Best regards,
Krzysztof

[PATCH 3/3] Documentation: mfd: s2mps11: Document support for S2MPS14

[Krzysztof Kozlowski]

Add bindings documentation for S2MPS14 device to the s2mps11 driver.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Tomasz Figa <t.figa@samsung.com>
Cc: devicetree@vger.kernel.org
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Acked-by: Tomasz Figa <t.figa@samsung.com>
---
 Documentation/devicetree/bindings/mfd/s2mps11.txt |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/s2mps11.txt b/Documentation/devicetree/bindings/mfd/s2mps11.txt
index 15ee89c3cc7b..f69bec294f02 100644
--- a/Documentation/devicetree/bindings/mfd/s2mps11.txt
+++ b/Documentation/devicetree/bindings/mfd/s2mps11.txt
@@ -1,5 +1,5 @@
 
-* Samsung S2MPS11 Voltage and Current Regulator
+* Samsung S2MPS11 and S2MPS14 Voltage and Current Regulator
 
 The Samsung S2MPS11 is a multi-function device which includes voltage and
 current regulators, RTC, charger controller and other sub-blocks. It is
@@ -7,7 +7,7 @@ interfaced to the host controller using an I2C interface. Each sub-block is
 addressed by the host system using different I2C slave addresses.
 
 Required properties:
-- compatible: Should be "samsung,s2mps11-pmic".
+- compatible: Should be "samsung,s2mps11-pmic" or "samsung,s2mps14-pmic".
 - reg: Specifies the I2C slave address of the pmic block. It should be 0x66.
 
 Optional properties:
@@ -59,10 +59,14 @@ supports. Note: The 'n' in LDOn and BUCKn represents the LDO or BUCK number
 as per the datasheet of s2mps11.
 
 	- LDOn
-		  - valid values for n are 1 to 38
+		  - valid values for n are:
+			- S2MPS11: 1 to 38
+			- S2MPS14: 1 to 25
 		  - Example: LDO1, LD02, LDO28
 	- BUCKn
-		  - valid values for n are 1 to 10.
+		  - valid values for n are:
+			- S2MPS11: 1 to 10
+			- S2MPS14: 1 to 5
 		  - Example: BUCK1, BUCK2, BUCK9
 
 Example:
-- 
1.7.9.5

Re: [PATCH 3/3] Documentation: mfd: s2mps11: Document support for S2MPS14

[Sachin Kamat]

Hi Krzysztof,

On 5 March 2014 14:52, Krzysztof Kozlowski <k.kozlowski@samsung.com> wrote:
> Add bindings documentation for S2MPS14 device to the s2mps11 driver.

nit: s/s2mps11/S2MPS11

>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Tomasz Figa <t.figa@samsung.com>
> Cc: devicetree@vger.kernel.org
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> Acked-by: Tomasz Figa <t.figa@samsung.com>
> ---
>  Documentation/devicetree/bindings/mfd/s2mps11.txt |   12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/s2mps11.txt b/Documentation/devicetree/bindings/mfd/s2mps11.txt
> index 15ee89c3cc7b..f69bec294f02 100644
> --- a/Documentation/devicetree/bindings/mfd/s2mps11.txt
> +++ b/Documentation/devicetree/bindings/mfd/s2mps11.txt
> @@ -1,5 +1,5 @@
>
> -* Samsung S2MPS11 Voltage and Current Regulator
> +* Samsung S2MPS11 and S2MPS14 Voltage and Current Regulator
>
>  The Samsung S2MPS11 is a multi-function device which includes voltage and
>  current regulators, RTC, charger controller and other sub-blocks. It is
> @@ -7,7 +7,7 @@ interfaced to the host controller using an I2C interface. Each sub-block is
>  addressed by the host system using different I2C slave addresses.
>
>  Required properties:
> -- compatible: Should be "samsung,s2mps11-pmic".
> +- compatible: Should be "samsung,s2mps11-pmic" or "samsung,s2mps14-pmic".
>  - reg: Specifies the I2C slave address of the pmic block. It should be 0x66.
>
>  Optional properties:
> @@ -59,10 +59,14 @@ supports. Note: The 'n' in LDOn and BUCKn represents the LDO or BUCK number
>  as per the datasheet of s2mps11.
>
>         - LDOn
> -                 - valid values for n are 1 to 38
> +                 - valid values for n are:
> +                       - S2MPS11: 1 to 38
> +                       - S2MPS14: 1 to 25
>                   - Example: LDO1, LD02, LDO28
>         - BUCKn
> -                 - valid values for n are 1 to 10.
> +                 - valid values for n are:
> +                       - S2MPS11: 1 to 10
> +                       - S2MPS14: 1 to 5
>                   - Example: BUCK1, BUCK2, BUCK9
>
>  Example:
> --
> 1.7.9.5
>
Looks good.
Acked-by: Sachin Kamat <sachin.kamat@linaro.org>


-- 
With warm regards,
Sachin