[PATCH v4 00/14] Add max77802 regulator operating mode support

[PATCH v4 00/14] Add max77802 regulator operating mode support

[Javier Martinez Canillas]

Hello Mark,

This is the fourth version of the series that adds operating modes
support for the regulators in the max77802 PMIC. This version uses
the standard suspend states bindings and the opmodes are parsed by
the regulator core while drivers only define a translation function
to map between hardware specific to standard modes as you suggested.

The series adds a "regulator-initial-mode" property to configure at
startup the operating mode for the regulators that support changing
its mode during normal operation and a "regulator-mode" property for
the regulators that supports changing its operating mode when the
system enters in a suspend state. These properties were originally
part of Chanwoo Choi's regulator suspend state series [0] but were
removed since there wasn't a way to define the operating modes in a
generic way.

In this series, the generic regulator DT binding doc explains that each
device has to document what their valid operating modes are. Drivers
must provide a translation function so the core can map the modes.

Since parsing the modes in the core is a very different approach, most
of the patches are new but those that remains have a changelog.

This series depend on [0] and also v2 of patch:
"ARM: EXYNOS: Call regulator core suspend prepare and finish functions" [1].

Javier Martinez Canillas (14):
  regulator: Document binding for initial and suspend modes
  regulator: Add function to map modes to struct regulator_desc
  regulator: of: Add regulator desc param to
    of_get_regulator_init_data()
  regulator: of: Pass the regulator description in the match table
  regulator: max1586: zero-initialize regulator match table array
  regulator: max77686: zero-initialize regulator match table
  regulator: max77802: zero-initialize regulator match table
  regulator: max8660: zero-initialize regulator match table array
  regulator: s2mpa01: zero-initialize regulator match table array
  regulator: of: Add support for parsing initial and suspend modes
  regulator: max77802: Document binding for regulator operating modes
  regulator: max77802: Use unsigned int for modes in max77802_map_mode()
  regulator: max77802: Set regulator modes translation callback
  ARM: dts: Configure regulators for suspend on exynos Peach boards

Patch #1 extends the regulator DT binding to document the initial and
suspend modes properties.

Patch #2 adds a function pointer to the static regulator description
so drivers can define a callback that does the modes translation.

Patch #3 does some refactoring to pass the regulator descriptor to the
function extracting the regulator initial data from DT.

Patch #4 adds a pointer to the regulator descriptor in the match table
so users extracting the init_data from of_regulator_match can also map
the modes.

Patch #5-#9 are fixes to be sure that all callers are passing an
initialised match table.

Patch #10 modifies the function that extracts the regulator data from
DT to parse the initial and suspend modes.

Patch #11 extends the max77802 DT binding to document the valid opmodes
for the regulators on this device.

Patch #12 change the signature of the function doing the modes mapping
for the max77802 regulators mode.

Patch #13 set the function handler for the max77802 modes translation.

Patch #14 configure the regulators for the Peach Pit and Pi Chromebooks.

Best regards,
Javier

[0]: https://lkml.org/lkml/2014/10/10/161
[1]: http://www.spinics.net/lists/arm-kernel/msg369923.html

[PATCH v4 01/14] regulator: Document binding for initial and suspend modes

[Javier Martinez Canillas]

Some regulators can run on different operating modes (opmodes). This
allows systems to choose the most efficient opmode for each regulator.

This patch builds on top of (291d761 regulator: Document binding for
regulator suspend state for PM state) adding a regulator-initial-mode
DT property to configure at startup the operating mode for regulators
that support changing its mode during normal operation and a property
regulator-mode to be used in the regulator-state-[mem/disk] nodes for
regulators that supports changing its operating mode when the system
enters in a suspend state.

The set of possible modes that a regulator can operate depends on the
hardware capabilities so a list of generic operating modes can't be
provided. Instead, each hardware binding should define the list of
valid operating modes for the regulators found on that device.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---

Changes in v3:
 - Rebased on top of regulator suspend voltage series

 Documentation/devicetree/bindings/regulator/regulator.txt | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
index 4e7ed76..3fffa3b 100644
--- a/Documentation/devicetree/bindings/regulator/regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/regulator.txt
@@ -30,6 +30,20 @@ Optional properties:
 	- regulator-off-in-suspend: regulator should be off in suspend state.
 	- regulator-suspend-microvolt: regulator should be set to this voltage
 	  in suspend.
+	- regulator-mode: operating mode in the given suspend state.
+	  The set of possible operating modes depends on the capabilities of
+	  every hardware so the valid modes are documented on each regulator
+	  device tree binding document.
+	  The "regulator-mode" property only takes effect if the regulator is
+	  enabled for the given suspend state using "regulator-on-in-suspend".
+	  If the regulator has not been explicitly disabled for the given state
+	  with "regulator-off-in-suspend", then setting the operating mode
+	  will also have no effect.
+- regulator-initial-mode: initial operating mode. The set of possible operating
+  modes is the same used for the regulator-mode property and the device binding
+  documentation explains which property each regulator supports.
+If no mode is defined, then the OS will not manage the modes and the hardware
+default values will be used instead.
 
 Deprecated properties:
 - regulator-compatible: If a regulator chip contains multiple
-- 
2.1.0

[PATCH v4 02/14] regulator: Add function to map modes to struct regulator_desc

[Javier Martinez Canillas]

The regulator-initial-mode and regulator-mode DT properties allows to
configure the regulator operating modes at startup or when a system
enters into a susend state.

But these properties use as valid values the operating modes supported
by each device while the core deals with the standard operating modes.
So a mapping function is needed to translate from the hardware specific
modes to the standard ones.

This mapping is a non-varying configuration for each regulator, so add
a function pointer to struct regulator_desc that will allow drivers to
define their callback to do the modes translation.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 include/linux/regulator/driver.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 28da08e..b54d037 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -243,6 +243,8 @@ enum regulator_type {
  *
  * @enable_time: Time taken for initial enable of regulator (in uS).
  * @off_on_delay: guard time (in uS), before re-enabling a regulator
+ *
+ * @map_modes: Callback invoked to translate between hardware to standard modes.
  */
 struct regulator_desc {
 	const char *name;
@@ -285,6 +287,8 @@ struct regulator_desc {
 	unsigned int enable_time;
 
 	unsigned int off_on_delay;
+
+	unsigned int (*map_modes)(unsigned int mode);
 };
 
 /**
-- 
2.1.0

[PATCH v4 03/14] regulator: of: Add regulator desc param to of_get_regulator_init_data()

[Javier Martinez Canillas]

The of_get_regulator_init_data() function is used to extract the regulator
init_data but information on how to extract certain data is defined in the
static regulator descriptor (e.g: how to map the hardware operating modes).

Add a const struct regulator_desc * parameter to the function signature so
the parsing logic could use the information in the struct regulator_desc.

of_get_regulator_init_data() relies on of_get_regulation_constraints() to
actually extract the init_data so it has to pass the struct regulator_desc
but that is changed in a following patch.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 drivers/regulator/88pm8607.c               | 3 ++-
 drivers/regulator/anatop-regulator.c       | 2 +-
 drivers/regulator/arizona-ldo1.c           | 3 ++-
 drivers/regulator/arizona-micsupp.c        | 2 +-
 drivers/regulator/da9052-regulator.c       | 2 +-
 drivers/regulator/da9210-regulator.c       | 2 +-
 drivers/regulator/fan53555.c               | 2 +-
 drivers/regulator/fixed.c                  | 2 +-
 drivers/regulator/gpio-regulator.c         | 2 +-
 drivers/regulator/max8952.c                | 2 +-
 drivers/regulator/max8973-regulator.c      | 3 ++-
 drivers/regulator/max8997.c                | 2 +-
 drivers/regulator/max8998.c                | 4 ++--
 drivers/regulator/mc13xxx-regulator-core.c | 3 ++-
 drivers/regulator/of_regulator.c           | 9 ++++++---
 drivers/regulator/pwm-regulator.c          | 2 +-
 drivers/regulator/qcom_rpm-regulator.c     | 3 ++-
 drivers/regulator/s5m8767.c                | 2 +-
 drivers/regulator/sky81452-regulator.c     | 2 +-
 drivers/regulator/stw481x-vmmc.c           | 2 +-
 drivers/regulator/ti-abb-regulator.c       | 2 +-
 drivers/regulator/tps51632-regulator.c     | 3 ++-
 drivers/regulator/tps62360-regulator.c     | 3 ++-
 drivers/regulator/tps65218-regulator.c     | 3 ++-
 drivers/regulator/twl-regulator.c          | 2 +-
 drivers/regulator/vexpress.c               | 3 ++-
 include/linux/regulator/of_regulator.h     | 8 ++++++--
 27 files changed, 47 insertions(+), 31 deletions(-)

diff --git a/drivers/regulator/88pm8607.c b/drivers/regulator/88pm8607.c
index 6d77dcd..ce0343a 100644
--- a/drivers/regulator/88pm8607.c
+++ b/drivers/regulator/88pm8607.c
@@ -330,7 +330,8 @@ static int pm8607_regulator_dt_init(struct platform_device *pdev,
 	for_each_child_of_node(nproot, np) {
 		if (!of_node_cmp(np->name, info->desc.name)) {
 			config->init_data =
-				of_get_regulator_init_data(&pdev->dev, np);
+				of_get_regulator_init_data(&pdev->dev, np,
+							   NULL);
 			config->of_node = np;
 			break;
 		}
diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c
index 542d14e..5cdce49 100644
--- a/drivers/regulator/anatop-regulator.c
+++ b/drivers/regulator/anatop-regulator.c
@@ -189,7 +189,7 @@ static int anatop_regulator_probe(struct platform_device *pdev)
 	int ret = 0;
 	u32 val;
 
-	initdata = of_get_regulator_init_data(dev, np);
+	initdata = of_get_regulator_init_data(dev, np, NULL);
 	sreg = devm_kzalloc(dev, sizeof(*sreg), GFP_KERNEL);
 	if (!sreg)
 		return -ENOMEM;
diff --git a/drivers/regulator/arizona-ldo1.c b/drivers/regulator/arizona-ldo1.c
index 4c9db58..fb393c5 100644
--- a/drivers/regulator/arizona-ldo1.c
+++ b/drivers/regulator/arizona-ldo1.c
@@ -194,7 +194,8 @@ static int arizona_ldo1_of_get_pdata(struct arizona *arizona,
 	if (init_node) {
 		config->of_node = init_node;
 
-		init_data = of_get_regulator_init_data(arizona->dev, init_node);
+		init_data = of_get_regulator_init_data(arizona->dev, init_node,
+						       NULL);
 
 		if (init_data) {
 			init_data->consumer_supplies = &ldo1->supply;
diff --git a/drivers/regulator/arizona-micsupp.c b/drivers/regulator/arizona-micsupp.c
index ce9aca5..71290bf 100644
--- a/drivers/regulator/arizona-micsupp.c
+++ b/drivers/regulator/arizona-micsupp.c
@@ -210,7 +210,7 @@ static int arizona_micsupp_of_get_pdata(struct arizona *arizona,
 	if (np) {
 		config->of_node = np;
 
-		init_data = of_get_regulator_init_data(arizona->dev, np);
+		init_data = of_get_regulator_init_data(arizona->dev, np, NULL);
 
 		if (init_data) {
 			init_data->consumer_supplies = &micsupp->supply;
diff --git a/drivers/regulator/da9052-regulator.c b/drivers/regulator/da9052-regulator.c
index 0003362..178343c 100644
--- a/drivers/regulator/da9052-regulator.c
+++ b/drivers/regulator/da9052-regulator.c
@@ -436,7 +436,7 @@ static int da9052_regulator_probe(struct platform_device *pdev)
 			if (!of_node_cmp(np->name,
 					 regulator->info->reg_desc.name)) {
 				config.init_data = of_get_regulator_init_data(
-					&pdev->dev, np);
+					&pdev->dev, np, NULL);
 				config.of_node = np;
 				break;
 			}
diff --git a/drivers/regulator/da9210-regulator.c b/drivers/regulator/da9210-regulator.c
index 7a320dd..aa9cab0 100644
--- a/drivers/regulator/da9210-regulator.c
+++ b/drivers/regulator/da9210-regulator.c
@@ -147,7 +147,7 @@ static int da9210_i2c_probe(struct i2c_client *i2c,
 
 	config.dev = &i2c->dev;
 	config.init_data = pdata ? &pdata->da9210_constraints :
-		of_get_regulator_init_data(dev, dev->of_node);
+		of_get_regulator_init_data(dev, dev->of_node, NULL);
 	config.driver_data = chip;
 	config.regmap = chip->regmap;
 	config.of_node = dev->of_node;
diff --git a/drivers/regulator/fan53555.c b/drivers/regulator/fan53555.c
index f8e4257..eb09c15 100644
--- a/drivers/regulator/fan53555.c
+++ b/drivers/regulator/fan53555.c
@@ -312,7 +312,7 @@ static struct fan53555_platform_data *fan53555_parse_dt(struct device *dev,
 	if (!pdata)
 		return NULL;
 
-	pdata->regulator = of_get_regulator_init_data(dev, np);
+	pdata->regulator = of_get_regulator_init_data(dev, np, NULL);
 
 	ret = of_property_read_u32(np, "fcs,suspend-voltage-selector",
 				   &tmp);
diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index 354105e..f8d4fe1 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -57,7 +57,7 @@ of_get_fixed_voltage_config(struct device *dev)
 	if (!config)
 		return ERR_PTR(-ENOMEM);
 
-	config->init_data = of_get_regulator_init_data(dev, dev->of_node);
+	config->init_data = of_get_regulator_init_data(dev, dev->of_node, NULL);
 	if (!config->init_data)
 		return ERR_PTR(-EINVAL);
 
diff --git a/drivers/regulator/gpio-regulator.c b/drivers/regulator/gpio-regulator.c
index 989b23b..a190ef4 100644
--- a/drivers/regulator/gpio-regulator.c
+++ b/drivers/regulator/gpio-regulator.c
@@ -146,7 +146,7 @@ of_get_gpio_regulator_config(struct device *dev, struct device_node *np)
 	if (!config)
 		return ERR_PTR(-ENOMEM);
 
-	config->init_data = of_get_regulator_init_data(dev, np);
+	config->init_data = of_get_regulator_init_data(dev, np, NULL);
 	if (!config->init_data)
 		return ERR_PTR(-EINVAL);
 
diff --git a/drivers/regulator/max8952.c b/drivers/regulator/max8952.c
index f7f9efc..ff4ba10 100644
--- a/drivers/regulator/max8952.c
+++ b/drivers/regulator/max8952.c
@@ -174,7 +174,7 @@ static struct max8952_platform_data *max8952_parse_dt(struct device *dev)
 	if (of_property_read_u32(np, "max8952,ramp-speed", &pd->ramp_speed))
 		dev_warn(dev, "max8952,ramp-speed property not specified, defaulting to 32mV/us\n");
 
-	pd->reg_data = of_get_regulator_init_data(dev, np);
+	pd->reg_data = of_get_regulator_init_data(dev, np, NULL);
 	if (!pd->reg_data) {
 		dev_err(dev, "Failed to parse regulator init data\n");
 		return NULL;
diff --git a/drivers/regulator/max8973-regulator.c b/drivers/regulator/max8973-regulator.c
index dbedf17..eb07d04 100644
--- a/drivers/regulator/max8973-regulator.c
+++ b/drivers/regulator/max8973-regulator.c
@@ -458,7 +458,8 @@ static int max8973_probe(struct i2c_client *client,
 
 	config.dev = &client->dev;
 	config.init_data = pdata ? pdata->reg_init_data :
-		of_get_regulator_init_data(&client->dev, client->dev.of_node);
+		of_get_regulator_init_data(&client->dev, client->dev.of_node,
+					   NULL);
 	config.driver_data = max;
 	config.of_node = client->dev.of_node;
 	config.regmap = max->regmap;
diff --git a/drivers/regulator/max8997.c b/drivers/regulator/max8997.c
index 9c31e21..b91526a 100644
--- a/drivers/regulator/max8997.c
+++ b/drivers/regulator/max8997.c
@@ -953,7 +953,7 @@ static int max8997_pmic_dt_parse_pdata(struct platform_device *pdev,
 
 		rdata->id = i;
 		rdata->initdata = of_get_regulator_init_data(&pdev->dev,
-							     reg_np);
+							     reg_np, NULL);
 		rdata->reg_node = reg_np;
 		rdata++;
 	}
diff --git a/drivers/regulator/max8998.c b/drivers/regulator/max8998.c
index 961091b..5608136 100644
--- a/drivers/regulator/max8998.c
+++ b/drivers/regulator/max8998.c
@@ -686,8 +686,8 @@ static int max8998_pmic_dt_parse_pdata(struct max8998_dev *iodev,
 			continue;
 
 		rdata->id = regulators[i].id;
-		rdata->initdata = of_get_regulator_init_data(
-							iodev->dev, reg_np);
+		rdata->initdata = of_get_regulator_init_data(iodev->dev,
+							     reg_np, NULL);
 		rdata->reg_node = reg_np;
 		++rdata;
 	}
diff --git a/drivers/regulator/mc13xxx-regulator-core.c b/drivers/regulator/mc13xxx-regulator-core.c
index afba024..838be1d 100644
--- a/drivers/regulator/mc13xxx-regulator-core.c
+++ b/drivers/regulator/mc13xxx-regulator-core.c
@@ -194,7 +194,8 @@ struct mc13xxx_regulator_init_data *mc13xxx_parse_regulators_dt(
 					 regulators[i].desc.name)) {
 				p->id = i;
 				p->init_data = of_get_regulator_init_data(
-							&pdev->dev, child);
+							&pdev->dev, child,
+							NULL);
 				p->node = child;
 				p++;
 
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index 03edb17..945486f 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -120,13 +120,16 @@ static void of_get_regulation_constraints(struct device_node *np,
 /**
  * of_get_regulator_init_data - extract regulator_init_data structure info
  * @dev: device requesting for regulator_init_data
+ * @node: regulator device node
+ * @desc: regulator description
  *
  * Populates regulator_init_data structure by extracting data from device
  * tree node, returns a pointer to the populated struture or NULL if memory
  * alloc fails.
  */
 struct regulator_init_data *of_get_regulator_init_data(struct device *dev,
-						struct device_node *node)
+					  struct device_node *node,
+					  const struct regulator_desc *desc)
 {
 	struct regulator_init_data *init_data;
 
@@ -218,7 +221,7 @@ int of_regulator_match(struct device *dev, struct device_node *node,
 				continue;
 
 			match->init_data =
-				of_get_regulator_init_data(dev, child);
+				of_get_regulator_init_data(dev, child, NULL);
 			if (!match->init_data) {
 				dev_err(dev,
 					"failed to parse DT for regulator %s\n",
@@ -266,7 +269,7 @@ struct regulator_init_data *regulator_of_get_init_data(struct device *dev,
 		if (strcmp(desc->of_match, name))
 			continue;
 
-		init_data = of_get_regulator_init_data(dev, child);
+		init_data = of_get_regulator_init_data(dev, child, desc);
 		if (!init_data) {
 			dev_err(dev,
 				"failed to parse DT for regulator %s\n",
diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index d3f55ea..dc4c678 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -149,7 +149,7 @@ static int pwm_regulator_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	config.init_data = of_get_regulator_init_data(&pdev->dev, np);
+	config.init_data = of_get_regulator_init_data(&pdev->dev, np, NULL);
 	if (!config.init_data)
 		return -ENOMEM;
 
diff --git a/drivers/regulator/qcom_rpm-regulator.c b/drivers/regulator/qcom_rpm-regulator.c
index b55cd5b..f1b7f27 100644
--- a/drivers/regulator/qcom_rpm-regulator.c
+++ b/drivers/regulator/qcom_rpm-regulator.c
@@ -643,7 +643,8 @@ static int rpm_reg_probe(struct platform_device *pdev)
 	match = of_match_device(rpm_of_match, &pdev->dev);
 	template = match->data;
 
-	initdata = of_get_regulator_init_data(&pdev->dev, pdev->dev.of_node);
+	initdata = of_get_regulator_init_data(&pdev->dev, pdev->dev.of_node,
+					      NULL);
 	if (!initdata)
 		return -EINVAL;
 
diff --git a/drivers/regulator/s5m8767.c b/drivers/regulator/s5m8767.c
index 0ab5cbe..6984e6b 100644
--- a/drivers/regulator/s5m8767.c
+++ b/drivers/regulator/s5m8767.c
@@ -581,7 +581,7 @@ static int s5m8767_pmic_dt_parse_pdata(struct platform_device *pdev,
 
 		rdata->id = i;
 		rdata->initdata = of_get_regulator_init_data(
-						&pdev->dev, reg_np);
+						&pdev->dev, reg_np, NULL);
 		rdata->reg_node = reg_np;
 		rdata++;
 		rmode->id = i;
diff --git a/drivers/regulator/sky81452-regulator.c b/drivers/regulator/sky81452-regulator.c
index 476b80a..6b3d3bf 100644
--- a/drivers/regulator/sky81452-regulator.c
+++ b/drivers/regulator/sky81452-regulator.c
@@ -76,7 +76,7 @@ static struct regulator_init_data *sky81452_reg_parse_dt(struct device *dev)
 		return NULL;
 	}
 
-	init_data = of_get_regulator_init_data(dev, np);
+	init_data = of_get_regulator_init_data(dev, np, NULL);
 
 	of_node_put(np);
 	return init_data;
diff --git a/drivers/regulator/stw481x-vmmc.c b/drivers/regulator/stw481x-vmmc.c
index a7e1526..51ff1869 100644
--- a/drivers/regulator/stw481x-vmmc.c
+++ b/drivers/regulator/stw481x-vmmc.c
@@ -72,7 +72,7 @@ static int stw481x_vmmc_regulator_probe(struct platform_device *pdev)
 	config.regmap = stw481x->map;
 	config.of_node = pdev->dev.of_node;
 	config.init_data = of_get_regulator_init_data(&pdev->dev,
-						      pdev->dev.of_node);
+						      pdev->dev.of_node, NULL);
 
 	stw481x->vmmc_regulator = devm_regulator_register(&pdev->dev,
 						&vmmc_regulator, &config);
diff --git a/drivers/regulator/ti-abb-regulator.c b/drivers/regulator/ti-abb-regulator.c
index a2dabb5..04f2421 100644
--- a/drivers/regulator/ti-abb-regulator.c
+++ b/drivers/regulator/ti-abb-regulator.c
@@ -837,7 +837,7 @@ skip_opt:
 		return -EINVAL;
 	}
 
-	initdata = of_get_regulator_init_data(dev, pdev->dev.of_node);
+	initdata = of_get_regulator_init_data(dev, pdev->dev.of_node, NULL);
 	if (!initdata) {
 		dev_err(dev, "%s: Unable to alloc regulator init data\n",
 			__func__);
diff --git a/drivers/regulator/tps51632-regulator.c b/drivers/regulator/tps51632-regulator.c
index f31f22e..65f63c4 100644
--- a/drivers/regulator/tps51632-regulator.c
+++ b/drivers/regulator/tps51632-regulator.c
@@ -230,7 +230,8 @@ static struct tps51632_regulator_platform_data *
 	if (!pdata)
 		return NULL;
 
-	pdata->reg_init_data = of_get_regulator_init_data(dev, dev->of_node);
+	pdata->reg_init_data = of_get_regulator_init_data(dev, dev->of_node,
+							  NULL);
 	if (!pdata->reg_init_data) {
 		dev_err(dev, "Not able to get OF regulator init data\n");
 		return NULL;
diff --git a/drivers/regulator/tps62360-regulator.c b/drivers/regulator/tps62360-regulator.c
index a167204..45c6b4c 100644
--- a/drivers/regulator/tps62360-regulator.c
+++ b/drivers/regulator/tps62360-regulator.c
@@ -302,7 +302,8 @@ static struct tps62360_regulator_platform_data *
 	if (!pdata)
 		return NULL;
 
-	pdata->reg_init_data = of_get_regulator_init_data(dev, dev->of_node);
+	pdata->reg_init_data = of_get_regulator_init_data(dev, dev->of_node,
+							  NULL);
 	if (!pdata->reg_init_data) {
 		dev_err(dev, "Not able to get OF regulator init data\n");
 		return NULL;
diff --git a/drivers/regulator/tps65218-regulator.c b/drivers/regulator/tps65218-regulator.c
index f0a4028..f3f56ea 100644
--- a/drivers/regulator/tps65218-regulator.c
+++ b/drivers/regulator/tps65218-regulator.c
@@ -231,7 +231,8 @@ static int tps65218_regulator_probe(struct platform_device *pdev)
 
 	template = match->data;
 	id = template->id;
-	init_data = of_get_regulator_init_data(&pdev->dev, pdev->dev.of_node);
+	init_data = of_get_regulator_init_data(&pdev->dev, pdev->dev.of_node,
+					       NULL);
 
 	platform_set_drvdata(pdev, tps);
 
diff --git a/drivers/regulator/twl-regulator.c b/drivers/regulator/twl-regulator.c
index 0b4f866..5d4fcb8 100644
--- a/drivers/regulator/twl-regulator.c
+++ b/drivers/regulator/twl-regulator.c
@@ -1104,7 +1104,7 @@ static int twlreg_probe(struct platform_device *pdev)
 		template = match->data;
 		id = template->desc.id;
 		initdata = of_get_regulator_init_data(&pdev->dev,
-						      pdev->dev.of_node);
+						      pdev->dev.of_node, NULL);
 		drvdata = NULL;
 	} else {
 		id = pdev->id;
diff --git a/drivers/regulator/vexpress.c b/drivers/regulator/vexpress.c
index 02e7267..b88e0ca 100644
--- a/drivers/regulator/vexpress.c
+++ b/drivers/regulator/vexpress.c
@@ -74,7 +74,8 @@ static int vexpress_regulator_probe(struct platform_device *pdev)
 	reg->desc.owner = THIS_MODULE;
 	reg->desc.continuous_voltage_range = true;
 
-	init_data = of_get_regulator_init_data(&pdev->dev, pdev->dev.of_node);
+	init_data = of_get_regulator_init_data(&pdev->dev, pdev->dev.of_node,
+					       NULL);
 	if (!init_data)
 		return -EINVAL;
 
diff --git a/include/linux/regulator/of_regulator.h b/include/linux/regulator/of_regulator.h
index f921796..3bbfb1b 100644
--- a/include/linux/regulator/of_regulator.h
+++ b/include/linux/regulator/of_regulator.h
@@ -6,6 +6,8 @@
 #ifndef __LINUX_OF_REG_H
 #define __LINUX_OF_REG_H
 
+#include <linux/regulator/driver.h>
+
 struct of_regulator_match {
 	const char *name;
 	void *driver_data;
@@ -16,14 +18,16 @@ struct of_regulator_match {
 #if defined(CONFIG_OF)
 extern struct regulator_init_data
 	*of_get_regulator_init_data(struct device *dev,
-				    struct device_node *node);
+				    struct device_node *node,
+				    const struct regulator_desc *desc);
 extern int of_regulator_match(struct device *dev, struct device_node *node,
 			      struct of_regulator_match *matches,
 			      unsigned int num_matches);
 #else
 static inline struct regulator_init_data
 	*of_get_regulator_init_data(struct device *dev,
-				    struct device_node *node)
+				    struct device_node *node,
+				    const struct regulator_desc *desc)
 {
 	return NULL;
 }
-- 
2.1.0

[PATCH v4 04/14] regulator: of: Pass the regulator description in the match table

[Javier Martinez Canillas]

Drivers can use the of_regulator_match() function to parse the regulator
init_data from DT. A match table is used to specify the name of the node
containing the regulators, the device node and to return the init_data
to the caller.

But also the static regulator descriptor is needed to correctly extract
some DT properties like the regulator initial and suspend modes. Use the
match table to pass that information.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 drivers/regulator/of_regulator.c       | 3 ++-
 include/linux/regulator/of_regulator.h | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index 945486f..cbc1d71 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -221,7 +221,8 @@ int of_regulator_match(struct device *dev, struct device_node *node,
 				continue;
 
 			match->init_data =
-				of_get_regulator_init_data(dev, child, NULL);
+				of_get_regulator_init_data(dev, child,
+							   match->desc);
 			if (!match->init_data) {
 				dev_err(dev,
 					"failed to parse DT for regulator %s\n",
diff --git a/include/linux/regulator/of_regulator.h b/include/linux/regulator/of_regulator.h
index 3bbfb1b..ce0877d 100644
--- a/include/linux/regulator/of_regulator.h
+++ b/include/linux/regulator/of_regulator.h
@@ -13,6 +13,7 @@ struct of_regulator_match {
 	void *driver_data;
 	struct regulator_init_data *init_data;
 	struct device_node *of_node;
+	const struct regulator_desc *desc;
 };
 
 #if defined(CONFIG_OF)
-- 
2.1.0

[PATCH v4 05/14] regulator: max1586: zero-initialize regulator match table array

[Javier Martinez Canillas]

The struct of_regulator_match rmatch[] is declared as a non-static local
variable so the structure members are not auto-initialized.

Initialize the array at declaration time to avoid the structure members
values to be indeterminate and have sane defaults instead.

Signed-off-by: Javier Martinez Canillas <javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
---
 drivers/regulator/max1586.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/max1586.c b/drivers/regulator/max1586.c
index 86db310..d2a8c64 100644
--- a/drivers/regulator/max1586.c
+++ b/drivers/regulator/max1586.c
@@ -163,7 +163,7 @@ static int of_get_max1586_platform_data(struct device *dev,
 				 struct max1586_platform_data *pdata)
 {
 	struct max1586_subdev_data *sub;
-	struct of_regulator_match rmatch[ARRAY_SIZE(max1586_reg)];
+	struct of_regulator_match rmatch[ARRAY_SIZE(max1586_reg)] = { };
 	struct device_node *np = dev->of_node;
 	int i, matched;
 
-- 
2.1.0

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

[PATCH v4 06/14] regulator: max77686: zero-initialize regulator match table

[Javier Martinez Canillas]

The struct of_regulator_match is declared as a non-static local variable
so the structure members are not auto-initialized.

Initialize the struct at declaration time to avoid the structure members
values to be indeterminate and have sane defaults instead.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 drivers/regulator/max77686.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/max77686.c b/drivers/regulator/max77686.c
index 09b0d8c..281eb4b 100644
--- a/drivers/regulator/max77686.c
+++ b/drivers/regulator/max77686.c
@@ -434,7 +434,7 @@ static int max77686_pmic_dt_parse_pdata(struct platform_device *pdev,
 	struct max77686_dev *iodev = dev_get_drvdata(pdev->dev.parent);
 	struct device_node *pmic_np, *regulators_np;
 	struct max77686_regulator_data *rdata;
-	struct of_regulator_match rmatch;
+	struct of_regulator_match rmatch = { };
 	unsigned int i;
 
 	pmic_np = iodev->dev->of_node;
-- 
2.1.0

[PATCH v4 07/14] regulator: max77802: zero-initialize regulator match table

[Javier Martinez Canillas]

The struct of_regulator_match is declared as a non-static local variable
so the structure members are not auto-initialized.

Initialize the struct at declaration time to avoid the structure members
values to be indeterminate and have sane defaults instead.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 drivers/regulator/max77802.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/max77802.c b/drivers/regulator/max77802.c
index a0d1462..d10c605 100644
--- a/drivers/regulator/max77802.c
+++ b/drivers/regulator/max77802.c
@@ -520,7 +520,7 @@ static int max77802_pmic_dt_parse_pdata(struct platform_device *pdev,
 	struct max77686_dev *iodev = dev_get_drvdata(pdev->dev.parent);
 	struct device_node *pmic_np, *regulators_np;
 	struct max77686_regulator_data *rdata;
-	struct of_regulator_match rmatch;
+	struct of_regulator_match rmatch = { };
 	unsigned int i;
 
 	pmic_np = iodev->dev->of_node;
-- 
2.1.0

[PATCH v4 08/14] regulator: max8660: zero-initialize regulator match table array

[Javier Martinez Canillas]

The struct of_regulator_match rmatch[] is declared as a non-static local
variable so the structure members are not auto-initialized.

Initialize the array at declaration time to avoid the structure members
values to be indeterminate and have sane defaults instead.

Signed-off-by: Javier Martinez Canillas <javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
---
 drivers/regulator/max8660.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/max8660.c b/drivers/regulator/max8660.c
index 2fc4111..7eee2ca 100644
--- a/drivers/regulator/max8660.c
+++ b/drivers/regulator/max8660.c
@@ -335,7 +335,7 @@ static int max8660_pdata_from_dt(struct device *dev,
 	int matched, i;
 	struct device_node *np;
 	struct max8660_subdev_data *sub;
-	struct of_regulator_match rmatch[ARRAY_SIZE(max8660_reg)];
+	struct of_regulator_match rmatch[ARRAY_SIZE(max8660_reg)] = { };
 
 	np = of_get_child_by_name(dev->of_node, "regulators");
 	if (!np) {
-- 
2.1.0

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

[PATCH v4 09/14] regulator: s2mpa01: zero-initialize regulator match table array

[Javier Martinez Canillas]

The struct of_regulator_match rmatch[] is declared as a non-static local
variable so the structure members are not auto-initialized.

Initialize the array at declaration time to avoid the structure members
values to be indeterminate and have sane defaults instead.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 drivers/regulator/s2mpa01.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/s2mpa01.c b/drivers/regulator/s2mpa01.c
index 2263071..5db4e12 100644
--- a/drivers/regulator/s2mpa01.c
+++ b/drivers/regulator/s2mpa01.c
@@ -341,7 +341,7 @@ static int s2mpa01_pmic_probe(struct platform_device *pdev)
 {
 	struct sec_pmic_dev *iodev = dev_get_drvdata(pdev->dev.parent);
 	struct sec_platform_data *pdata = dev_get_platdata(iodev->dev);
-	struct of_regulator_match rdata[S2MPA01_REGULATOR_MAX];
+	struct of_regulator_match rdata[S2MPA01_REGULATOR_MAX] = { };
 	struct device_node *reg_np = NULL;
 	struct regulator_config config = { };
 	struct s2mpa01_info *s2mpa01;
-- 
2.1.0

[PATCH v4 10/14] regulator: of: Add support for parsing initial and suspend modes

[Javier Martinez Canillas]

Some regulators support their operating mode to be changed on startup
or by consumers when the system is running while others only support
their operating mode to be changed while the system has entered in a
suspend state.

The regulator Device Tree binding documents a set of properties to
configure the regulators operating modes from a FDT. This patch builds
on (40e20d6 regulator: of: Add support for parsing regulator_state for
suspend state) and adds support to parse those properties and fill the
regulator constraints so the regulator core can call the right suspend
handlers when the system enters into sleep.

The modes are defined in the Device Tree using the hardware specific
modes supported by the regulators. Regulator drivers have to define a
translation function that is used to map the hardware specific modes
to the standard ones.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---

Changes in v4:
 - Parse the properties in the core and map using driver provided functions.
   Suggested by Mark Brown

Changes in v3:
 - Use the standard suspend states binding instead of custom properties.
   Suggested by Mark Brown

 drivers/regulator/of_regulator.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index cbc1d71..cd65885 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -25,7 +25,8 @@ static const char *const regulator_states[PM_SUSPEND_MAX + 1] = {
 };
 
 static void of_get_regulation_constraints(struct device_node *np,
-					struct regulator_init_data **init_data)
+					struct regulator_init_data **init_data,
+					const struct regulator_desc *desc)
 {
 	const __be32 *min_uV, *max_uV;
 	struct regulation_constraints *constraints = &(*init_data)->constraints;
@@ -81,6 +82,14 @@ static void of_get_regulation_constraints(struct device_node *np,
 	if (!ret)
 		constraints->enable_time = pval;
 
+	if (!of_property_read_u32(np, "regulator-initial-mode", &pval)) {
+		if (desc && desc->map_modes)
+			constraints->initial_mode = desc->map_modes(pval);
+		else
+			pr_warn("%s: failed to parse regulator-initial-mode\n",
+				np->name);
+	}
+
 	for (i = 0; i < ARRAY_SIZE(regulator_states); i++) {
 		switch (i) {
 		case PM_SUSPEND_MEM:
@@ -100,6 +109,15 @@ static void of_get_regulation_constraints(struct device_node *np,
 		if (!suspend_np || !suspend_state)
 			continue;
 
+		if (!of_property_read_u32(suspend_np, "regulator-mode",
+					  &pval)) {
+			if (desc && desc->map_modes)
+				suspend_state->mode = desc->map_modes(pval);
+			else
+				pr_warn("%s: failed to parse regulator-mode\n",
+					np->name);
+		}
+
 		if (of_property_read_bool(suspend_np,
 					"regulator-on-in-suspend"))
 			suspend_state->enabled = true;
@@ -140,7 +158,7 @@ struct regulator_init_data *of_get_regulator_init_data(struct device *dev,
 	if (!init_data)
 		return NULL; /* Out of memory? */
 
-	of_get_regulation_constraints(node, &init_data);
+	of_get_regulation_constraints(node, &init_data, desc);
 	return init_data;
 }
 EXPORT_SYMBOL_GPL(of_get_regulator_init_data);
-- 
2.1.0

[PATCH v4 11/14] regulator: max77802: Document binding for regulator operating modes

[Javier Martinez Canillas]

Some regulators from the max77802 PMIC support to be configured in one
of two operating mode: Output ON (normal) and Output On Low Power Mode.
Not all regulators support these two modes and for some of them, the
mode can be changed while the system is running in normal operation
while others only support their mode to be changed on system suspend.

Extend the max77802 PMIC bindin by documenting the possible operating
modes values so the regulators modes can be correctly configured.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---

Changes in v4: None

Changes in v3:
 - Use the standard suspend states bindings as suggested by Mark Brown.

 .../devicetree/bindings/regulator/max77802.txt     | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/max77802.txt b/Documentation/devicetree/bindings/regulator/max77802.txt
index 5aeaffc..1a78ec2 100644
--- a/Documentation/devicetree/bindings/regulator/max77802.txt
+++ b/Documentation/devicetree/bindings/regulator/max77802.txt
@@ -25,6 +25,27 @@ with their hardware counterparts as follow. The valid names are:
 			example: LDO1, LDO2, LDO35.
 	-BUCKn 	:	for BUCKs, where n can lie in range 1 to 10.
 			example: BUCK1, BUCK5, BUCK10.
+
+The max77802 regulator supports two different operating modes: Normal and Low
+Power Mode. Some regulators support the modes to be changed at startup or by
+the consumers during normal operation while others only support to change the
+mode during system suspend. The standard regulator suspend states binding can
+be used to configure the regulator operating mode.
+
+The regulators that support the standard "regulator-initial-mode" property,
+changing their mode during normal operation are: LDOs 1, 3, 20 and 21.
+
+The possible values for "regulator-initial-mode" and "regulator-mode" are:
+	1: Normal regulator voltage output mode.
+	3: Low Power which reduces the quiescent current down to only 1uA
+
+The list of valid modes are defined in the dt-bindings/regulator/regulator.h
+header and can be included by device tree source files.
+
+The standard "regulator-mode" property can only be used for regulators that
+support changing their mode to Low Power Mode during suspend. These regulators
+are: BUCKs 2-4 and LDOs 1-35.
+
 Example:
 
 	max77802@09 {
@@ -36,11 +57,23 @@ Example:
 		#size-cells = <0>;
 
 		regulators {
+			ldo1_reg: LDO1 {
+				regulator-name = "vdd_1v0";
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <1000000>;
+				regulator-always-on;
+				regulator-initial-mode = <MAX77802_OPMODE_LP>;
+			};
+
 			ldo11_reg: LDO11 {
 				regulator-name = "vdd_ldo11";
 				regulator-min-microvolt = <1900000>;
 				regulator-max-microvolt = <1900000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-mode = <MAX77802_OPMODE_LP>;
+				};
 			};
 
 			buck1_reg: BUCK1 {
-- 
2.1.0

[PATCH v4 12/14] regulator: max77802: Use unsigned int for modes in max77802_map_mode()

[Javier Martinez Canillas]

All function dealing with operating modes use unsigned int for modes
so change max77802_map_mode() function signature for consistency.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 drivers/regulator/max77802.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/max77802.c b/drivers/regulator/max77802.c
index d10c605..1d1f7b4 100644
--- a/drivers/regulator/max77802.c
+++ b/drivers/regulator/max77802.c
@@ -73,7 +73,7 @@ struct max77802_regulator_prv {
 	unsigned int opmode[MAX77802_REG_MAX];
 };
 
-static inline int max77802_map_mode(int mode)
+static inline unsigned int max77802_map_mode(unsigned int mode)
 {
 	return mode == MAX77802_OPMODE_NORMAL ?
 		REGULATOR_MODE_NORMAL : REGULATOR_MODE_STANDBY;
-- 
2.1.0

[PATCH v4 13/14] regulator: max77802: Set regulator modes translation callback

[Javier Martinez Canillas]

The max77802 PMIC regulators output can be configured in one of two
modes: Output ON (normal) and Output ON in Low Power Mode. Some of
the regulators support their operating mode to be changed on startup
or by consumers when the system is running while others only support
their operating mode to be changed while the system has entered in a
suspend state.

Use the max77802_map_mode() function to translate the device specific
modes to the standard operating modes as used by the regulator core.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 drivers/regulator/max77802.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/regulator/max77802.c b/drivers/regulator/max77802.c
index 1d1f7b4..09276c2 100644
--- a/drivers/regulator/max77802.c
+++ b/drivers/regulator/max77802.c
@@ -375,6 +375,7 @@ static struct regulator_ops max77802_buck_dvs_ops = {
 	.vsel_mask	= MAX77802_VSEL_MASK,				\
 	.enable_reg	= MAX77802_REG_LDO1CTRL1 + num - 1,		\
 	.enable_mask	= MAX77802_OPMODE_MASK << MAX77802_OPMODE_SHIFT_LDO, \
+	.map_modes	= max77802_map_mode,				\
 }
 
 /* LDOs 1, 2, 8, 15, 17, 27, 30, 35 */
@@ -393,6 +394,7 @@ static struct regulator_ops max77802_buck_dvs_ops = {
 	.vsel_mask	= MAX77802_VSEL_MASK,				\
 	.enable_reg	= MAX77802_REG_LDO1CTRL1 + num - 1,		\
 	.enable_mask	= MAX77802_OPMODE_MASK << MAX77802_OPMODE_SHIFT_LDO, \
+	.map_modes	= max77802_map_mode,				\
 }
 
 /* BUCKs 1, 6 */
@@ -411,6 +413,7 @@ static struct regulator_ops max77802_buck_dvs_ops = {
 	.vsel_mask	= MAX77802_DVS_VSEL_MASK,			\
 	.enable_reg	= MAX77802_REG_BUCK ## num ## CTRL,		\
 	.enable_mask	= MAX77802_OPMODE_MASK,				\
+	.map_modes	= max77802_map_mode,				\
 }
 
 /* BUCKS 2-4 */
@@ -430,6 +433,7 @@ static struct regulator_ops max77802_buck_dvs_ops = {
 	.enable_reg	= MAX77802_REG_BUCK ## num ## CTRL1,		\
 	.enable_mask	= MAX77802_OPMODE_MASK <<			\
 				MAX77802_OPMODE_BUCK234_SHIFT,		\
+	.map_modes	= max77802_map_mode,				\
 }
 
 /* BUCK 5 */
@@ -448,6 +452,7 @@ static struct regulator_ops max77802_buck_dvs_ops = {
 	.vsel_mask	= MAX77802_VSEL_MASK,				\
 	.enable_reg	= MAX77802_REG_BUCK5CTRL,			\
 	.enable_mask	= MAX77802_OPMODE_MASK,				\
+	.map_modes	= max77802_map_mode,				\
 }
 
 /* BUCKs 7-10 */
@@ -466,6 +471,7 @@ static struct regulator_ops max77802_buck_dvs_ops = {
 	.vsel_mask	= MAX77802_VSEL_MASK,				\
 	.enable_reg	= MAX77802_REG_BUCK7CTRL + (num - 7) * 3,	\
 	.enable_mask	= MAX77802_OPMODE_MASK,				\
+	.map_modes	= max77802_map_mode,				\
 }
 
 static const struct regulator_desc regulators[] = {
@@ -540,6 +546,7 @@ static int max77802_pmic_dt_parse_pdata(struct platform_device *pdev,
 
 	for (i = 0; i < pdata->num_regulators; i++) {
 		rmatch.name = regulators[i].name;
+		rmatch.desc = &regulators[i];
 		rmatch.init_data = NULL;
 		rmatch.of_node = NULL;
 		if (of_regulator_match(&pdev->dev, regulators_np, &rmatch,
-- 
2.1.0

[PATCH v4 14/14] ARM: dts: Configure regulators for suspend on exynos Peach boards

[Javier Martinez Canillas]

The regulator core now has support to choose if a regulator
has to be enabled or disabled during system suspend and also
the supports changing the regulator operating mode during
runtime and when the system enters into sleep mode.

To lower power during suspend, configure the regulators state
using the same configuration found in the ChromeOS 3.8 kernel.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---

Changes in v4: None

Changes in v3:
 - Use the standard suspend state binding as suggested by Mark Brown.

 arch/arm/boot/dts/exynos5420-peach-pit.dts | 81 ++++++++++++++++++++++++++++++
 arch/arm/boot/dts/exynos5800-peach-pi.dts  | 81 ++++++++++++++++++++++++++++++
 2 files changed, 162 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5420-peach-pit.dts b/arch/arm/boot/dts/exynos5420-peach-pit.dts
index 9a050e1..8b744c7 100644
--- a/arch/arm/boot/dts/exynos5420-peach-pit.dts
+++ b/arch/arm/boot/dts/exynos5420-peach-pit.dts
@@ -13,6 +13,7 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/clock/maxim,max77802.h>
+#include <dt-bindings/regulator/maxim,max77802.h>
 #include "exynos5420.dtsi"
 
 / {
@@ -192,6 +193,9 @@
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck2_reg: BUCK2 {
@@ -201,6 +205,9 @@
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck3_reg: BUCK3 {
@@ -210,6 +217,9 @@
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck4_reg: BUCK4 {
@@ -219,6 +229,9 @@
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck5_reg: BUCK5 {
@@ -227,6 +240,9 @@
 				regulator-max-microvolt = <1200000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck6_reg: BUCK6 {
@@ -236,6 +252,9 @@
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck7_reg: BUCK7 {
@@ -244,6 +263,9 @@
 				regulator-max-microvolt = <1350000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			buck8_reg: BUCK8 {
@@ -252,6 +274,9 @@
 				regulator-max-microvolt = <2850000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck9_reg: BUCK9 {
@@ -260,6 +285,9 @@
 				regulator-max-microvolt = <2000000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			buck10_reg: BUCK10 {
@@ -268,6 +296,9 @@
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			ldo1_reg: LDO1 {
@@ -275,6 +306,10 @@
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-mode = <MAX77802_OPMODE_LP>;
+				};
 			};
 
 			ldo2_reg: LDO2 {
@@ -288,6 +323,10 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-mode = <MAX77802_OPMODE_LP>;
+				};
 			};
 
 			vqmmc_sdcard: ldo4_reg: LDO4 {
@@ -295,6 +334,9 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <2800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo5_reg: LDO5 {
@@ -302,6 +344,9 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo6_reg: LDO6 {
@@ -309,6 +354,9 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo7_reg: LDO7 {
@@ -322,6 +370,9 @@
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo9_reg: LDO9 {
@@ -329,6 +380,10 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-mode = <MAX77802_OPMODE_LP>;
+				};
 			};
 
 			ldo10_reg: LDO10 {
@@ -336,6 +391,9 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo11_reg: LDO11 {
@@ -343,6 +401,10 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-mode = <MAX77802_OPMODE_LP>;
+				};
 			};
 
 			ldo12_reg: LDO12 {
@@ -350,6 +412,9 @@
 				regulator-min-microvolt = <3000000>;
 				regulator-max-microvolt = <3000000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo13_reg: LDO13 {
@@ -357,6 +422,10 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-mode = <MAX77802_OPMODE_LP>;
+				};
 			};
 
 			ldo14_reg: LDO14 {
@@ -364,6 +433,9 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo15_reg: LDO15 {
@@ -371,6 +443,9 @@
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo17_reg: LDO17 {
@@ -378,6 +453,9 @@
 				regulator-min-microvolt = <900000>;
 				regulator-max-microvolt = <1400000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo18_reg: LDO18 {
@@ -451,6 +529,9 @@
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo32_reg: LDO32 {
diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts b/arch/arm/boot/dts/exynos5800-peach-pi.dts
index e8fdda8..df7fbde 100644
--- a/arch/arm/boot/dts/exynos5800-peach-pi.dts
+++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts
@@ -13,6 +13,7 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/clock/maxim,max77802.h>
+#include <dt-bindings/regulator/maxim,max77802.h>
 #include "exynos5800.dtsi"
 
 / {
@@ -191,6 +192,9 @@
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck2_reg: BUCK2 {
@@ -200,6 +204,9 @@
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck3_reg: BUCK3 {
@@ -209,6 +216,9 @@
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck4_reg: BUCK4 {
@@ -218,6 +228,9 @@
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck5_reg: BUCK5 {
@@ -226,6 +239,9 @@
 				regulator-max-microvolt = <1200000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck6_reg: BUCK6 {
@@ -235,6 +251,9 @@
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck7_reg: BUCK7 {
@@ -243,6 +262,9 @@
 				regulator-max-microvolt = <1350000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			buck8_reg: BUCK8 {
@@ -251,6 +273,9 @@
 				regulator-max-microvolt = <2850000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck9_reg: BUCK9 {
@@ -259,6 +284,9 @@
 				regulator-max-microvolt = <2000000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			buck10_reg: BUCK10 {
@@ -267,6 +295,9 @@
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			ldo1_reg: LDO1 {
@@ -274,6 +305,10 @@
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-mode = <MAX77802_OPMODE_LP>;
+				};
 			};
 
 			ldo2_reg: LDO2 {
@@ -287,6 +322,10 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-mode = <MAX77802_OPMODE_LP>;
+				};
 			};
 
 			vqmmc_sdcard: ldo4_reg: LDO4 {
@@ -294,6 +333,9 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <2800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo5_reg: LDO5 {
@@ -301,6 +343,9 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo6_reg: LDO6 {
@@ -308,6 +353,9 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo7_reg: LDO7 {
@@ -321,6 +369,9 @@
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo9_reg: LDO9 {
@@ -328,6 +379,10 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-mode = <MAX77802_OPMODE_LP>;
+				};
 			};
 
 			ldo10_reg: LDO10 {
@@ -335,6 +390,9 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo11_reg: LDO11 {
@@ -342,6 +400,10 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-mode = <MAX77802_OPMODE_LP>;
+				};
 			};
 
 			ldo12_reg: LDO12 {
@@ -349,6 +411,9 @@
 				regulator-min-microvolt = <3000000>;
 				regulator-max-microvolt = <3000000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo13_reg: LDO13 {
@@ -356,6 +421,10 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-mode = <MAX77802_OPMODE_LP>;
+				};
 			};
 
 			ldo14_reg: LDO14 {
@@ -363,6 +432,9 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo15_reg: LDO15 {
@@ -370,6 +442,9 @@
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo17_reg: LDO17 {
@@ -377,6 +452,9 @@
 				regulator-min-microvolt = <900000>;
 				regulator-max-microvolt = <1400000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo18_reg: LDO18 {
@@ -450,6 +528,9 @@
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo32_reg: LDO32 {
-- 
2.1.0

Re: [PATCH v4 03/14] regulator: of: Add regulator desc param to of_get_regulator_init_data()

[Mark Brown]

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

On Mon, Nov 03, 2014 at 03:40:38PM +0100, Javier Martinez Canillas wrote:

>  	for_each_child_of_node(nproot, np) {
>  		if (!of_node_cmp(np->name, info->desc.name)) {
>  			config->init_data =
> -				of_get_regulator_init_data(&pdev->dev, np);
> +				of_get_regulator_init_data(&pdev->dev, np,
> +							   NULL);

This looks buggy, we're not passing in a descriptor.

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

Re: [PATCH v4 05/14] regulator: max1586: zero-initialize regulator match table array

[Mark Brown]

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

On Mon, Nov 03, 2014 at 03:40:40PM +0100, Javier Martinez Canillas wrote:
> The struct of_regulator_match rmatch[] is declared as a non-static local
> variable so the structure members are not auto-initialized.

Applied, thanks.

This is a bug fix not *that* closely related to the rest of the series,
if it's being included in a series like this it should've been at the
start of the series not after substantial new feature additions so that
the fixes don't get ignored due to problems with the features and so
that the fixes can be sent to Linus without dependencies.

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

Re: [PATCH v4 03/14] regulator: of: Add regulator desc param to of_get_regulator_init_data()

[Javier Martinez Canillas]

On 11/03/2014 04:33 PM, Mark Brown wrote:
> On Mon, Nov 03, 2014 at 03:40:38PM +0100, Javier Martinez Canillas wrote:
> 
>>  	for_each_child_of_node(nproot, np) {
>>  		if (!of_node_cmp(np->name, info->desc.name)) {
>>  			config->init_data =
>> -				of_get_regulator_init_data(&pdev->dev, np);
>> +				of_get_regulator_init_data(&pdev->dev, np,
>> +							   NULL);
> 
> This looks buggy, we're not passing in a descriptor.
> 

The descriptor is only used when extracting the init_data to map the
modes and since it was not a parameter before, some drivers needs to
be refactored to pass that information.

I thought that instead of adding intrusive changes in drivers that I
don't have hw to test, that parameter could be optional so the patch
that use that information check if the descriptor and the map_modes
function pointer are not NULL.

Best regards,
Javier

Re: [PATCH v4 05/14] regulator: max1586: zero-initialize regulator match table array

[Javier Martinez Canillas]

On 11/03/2014 04:41 PM, Mark Brown wrote:
> On Mon, Nov 03, 2014 at 03:40:40PM +0100, Javier Martinez Canillas wrote:
>> The struct of_regulator_match rmatch[] is declared as a non-static local
>> variable so the structure members are not auto-initialized.
> 
> Applied, thanks.
> 
> This is a bug fix not *that* closely related to the rest of the series,
> if it's being included in a series like this it should've been at the
> start of the series not after substantial new feature additions so that
> the fixes don't get ignored due to problems with the features and so
> that the fixes can be sent to Linus without dependencies.
> 

True, I added in the series since of_get_regulation_constraints() checks
if the struct regulator_desc * pointer is not NULL and so drivers that
don't initialize the struct of_regulator_match containing that pointer
will make that check to fail and the pointer be wrongly dereferenced.

Sorry, next time I'll post fixes separately and make the series depend on
that instead.

Best regards,
Javier

Re: [PATCH v4 00/14] Add max77802 regulator operating mode support

[Mark Brown]

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

On Mon, Nov 03, 2014 at 03:40:35PM +0100, Javier Martinez Canillas wrote:
> Hello Mark,
> 
> This is the fourth version of the series that adds operating modes
> support for the regulators in the max77802 PMIC. This version uses

No, it's not.  This is a a patch series doing a whole bunch of different
things, there's at least bug fixes to existing drivers, new features and
also this new driver in what I've glanced at so far.  These things
shouldn't just be being thrown together into a single patch series, and
the patch series shouldn't then be described as just being what is in
the end a minor part of the collection.

Doing this makes things more manageable from the review side, avoids
pointless dependencies and avoids setting off alarm bells.  My first
thought when seeing this was "how can a regulator driver be so complex
as to need 14 patches, there must be something seriously wrong here".

The whole thing about making sure that what you're doing makes sense
beyond just giving the correct test results also applies to sending the
patches - think if what's being said to the people reviewing the patches
is sensible.

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

Re: [PATCH v4 06/14] regulator: max77686: zero-initialize regulator match table

[Mark Brown]

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

On Mon, Nov 03, 2014 at 03:40:41PM +0100, Javier Martinez Canillas wrote:
> The struct of_regulator_match is declared as a non-static local variable
> so the structure members are not auto-initialized.

Applied, thanks.

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

Re: [PATCH v4 07/14] regulator: max77802: zero-initialize regulator match table

[Mark Brown]

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

On Mon, Nov 03, 2014 at 03:40:42PM +0100, Javier Martinez Canillas wrote:
> The struct of_regulator_match is declared as a non-static local variable
> so the structure members are not auto-initialized.

Applied, thanks.

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

Re: [PATCH v4 08/14] regulator: max8660: zero-initialize regulator match table array

[Mark Brown]

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

On Mon, Nov 03, 2014 at 03:40:43PM +0100, Javier Martinez Canillas wrote:
> The struct of_regulator_match rmatch[] is declared as a non-static local
> variable so the structure members are not auto-initialized.

Applied, thanks.

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

Re: [PATCH v4 09/14] regulator: s2mpa01: zero-initialize regulator match table array

[Mark Brown]

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

On Mon, Nov 03, 2014 at 03:40:44PM +0100, Javier Martinez Canillas wrote:
> The struct of_regulator_match rmatch[] is declared as a non-static local
> variable so the structure members are not auto-initialized.

Applied, thanks.

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

Re: [PATCH v4 03/14] regulator: of: Add regulator desc param to of_get_regulator_init_data()

[Mark Brown]

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

On Mon, Nov 03, 2014 at 04:48:34PM +0100, Javier Martinez Canillas wrote:
> On 11/03/2014 04:33 PM, Mark Brown wrote:
> > On Mon, Nov 03, 2014 at 03:40:38PM +0100, Javier Martinez Canillas wrote:

> >>  		if (!of_node_cmp(np->name, info->desc.name)) {
> >>  			config->init_data =
> >> -				of_get_regulator_init_data(&pdev->dev, np);
> >> +				of_get_regulator_init_data(&pdev->dev, np,
> >> +							   NULL);

> > This looks buggy, we're not passing in a descriptor.

> The descriptor is only used when extracting the init_data to map the
> modes and since it was not a parameter before, some drivers needs to
> be refactored to pass that information.

No, it's only *currently* used for that.  If we don't bother passing the
descriptor in then future additions which make use of it (including
adding mode operations to existing drivers) won't work and it might not
be obvious why.

> I thought that instead of adding intrusive changes in drivers that I
> don't have hw to test, that parameter could be optional so the patch
> that use that information check if the descriptor and the map_modes
> function pointer are not NULL.

You're already going through and modifying every single driver and all
of those I looked at already had references to the descriptor in
adjacent code or a global descriptor for the one regulator supported by
the driver.

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

Re: [PATCH v4 00/14] Add max77802 regulator operating mode support

[Mark Brown]

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

On Mon, Nov 03, 2014 at 03:54:43PM +0000, Mark Brown wrote:
> On Mon, Nov 03, 2014 at 03:40:35PM +0100, Javier Martinez Canillas wrote:
> > Hello Mark,
> > 
> > This is the fourth version of the series that adds operating modes
> > support for the regulators in the max77802 PMIC. This version uses

> No, it's not.  This is a a patch series doing a whole bunch of different
> things, there's at least bug fixes to existing drivers, new features and
> also this new driver in what I've glanced at so far.  These things

s/new driver/new driver support/ sorry

Point being that operating mode support in the driver is mostly
orthogonal to DT bindings is mostly orthogonal to random bug fixes.

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

Re: [PATCH v4 00/14] Add max77802 regulator operating mode support

[Javier Martinez Canillas]

On 11/03/2014 04:54 PM, Mark Brown wrote:
> 
> No, it's not.  This is a a patch series doing a whole bunch of different
> things, there's at least bug fixes to existing drivers, new features and
> also this new driver in what I've glanced at so far.  These things
> shouldn't just be being thrown together into a single patch series, and
> the patch series shouldn't then be described as just being what is in
> the end a minor part of the collection.
> 

Ok, sorry. Next time I'll take care to split fixes from new features and
also better explain what the series really does.

Best regards,
Javier

Re: [PATCH v4 12/14] regulator: max77802: Use unsigned int for modes in max77802_map_mode()

[Mark Brown]

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

On Mon, Nov 03, 2014 at 03:40:47PM +0100, Javier Martinez Canillas wrote:
> All function dealing with operating modes use unsigned int for modes
> so change max77802_map_mode() function signature for consistency.

Applied, thanks.

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

Re: [PATCH v4 03/14] regulator: of: Add regulator desc param to of_get_regulator_init_data()

[Javier Martinez Canillas]

On 11/03/2014 04:59 PM, Mark Brown wrote:
> No, it's only *currently* used for that.  If we don't bother passing the
> descriptor in then future additions which make use of it (including
> adding mode operations to existing drivers) won't work and it might not
> be obvious why.
>

fair enough.
 
> You're already going through and modifying every single driver and all
> of those I looked at already had references to the descriptor in
> adjacent code or a global descriptor for the one regulator supported by
> the driver.
> 

Ok, I'll make sure that all drivers passes the descriptor.

Best regards,
Javier

Re: [PATCH v4 02/14] regulator: Add function to map modes to struct regulator_desc

[Krzysztof Kozlowski]

On pon, 2014-11-03 at 15:40 +0100, Javier Martinez Canillas wrote:
> The regulator-initial-mode and regulator-mode DT properties allows to
> configure the regulator operating modes at startup or when a system
> enters into a susend state.
> 
> But these properties use as valid values the operating modes supported
> by each device while the core deals with the standard operating modes.
> So a mapping function is needed to translate from the hardware specific
> modes to the standard ones.
> 
> This mapping is a non-varying configuration for each regulator, so add
> a function pointer to struct regulator_desc that will allow drivers to
> define their callback to do the modes translation.
> 
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
>  include/linux/regulator/driver.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
> index 28da08e..b54d037 100644
> --- a/include/linux/regulator/driver.h
> +++ b/include/linux/regulator/driver.h
> @@ -243,6 +243,8 @@ enum regulator_type {
>   *
>   * @enable_time: Time taken for initial enable of regulator (in uS).
>   * @off_on_delay: guard time (in uS), before re-enabling a regulator
> + *
> + * @map_modes: Callback invoked to translate between hardware to standard modes.

Initially I thought it should map from standard to hardware. But then I
looked at max77802 implementation and it maps from hardware to standard.
Anyway I got confused (both are "modes" and both unsigned ints).

Could you describe which should be returned?


>   */
>  struct regulator_desc {
>  	const char *name;
> @@ -285,6 +287,8 @@ struct regulator_desc {
>  	unsigned int enable_time;
>  
>  	unsigned int off_on_delay;
> +
> +	unsigned int (*map_modes)(unsigned int mode);

Shouldn't this be in regulator ops?

Best regards,
Krzysztof

Re: [PATCH v4 10/14] regulator: of: Add support for parsing initial and suspend modes

[Krzysztof Kozlowski]

On pon, 2014-11-03 at 15:40 +0100, Javier Martinez Canillas wrote:
> Some regulators support their operating mode to be changed on startup
> or by consumers when the system is running while others only support
> their operating mode to be changed while the system has entered in a
> suspend state.
> 
> The regulator Device Tree binding documents a set of properties to
> configure the regulators operating modes from a FDT. This patch builds
> on (40e20d6 regulator: of: Add support for parsing regulator_state for
> suspend state) and adds support to parse those properties and fill the
> regulator constraints so the regulator core can call the right suspend
> handlers when the system enters into sleep.
> 
> The modes are defined in the Device Tree using the hardware specific
> modes supported by the regulators. Regulator drivers have to define a
> translation function that is used to map the hardware specific modes
> to the standard ones.
> 
> Signed-off-by: Javier Martinez Canillas <javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
> ---
> 
> Changes in v4:
>  - Parse the properties in the core and map using driver provided functions.
>    Suggested by Mark Brown
> 
> Changes in v3:
>  - Use the standard suspend states binding instead of custom properties.
>    Suggested by Mark Brown
> 
>  drivers/regulator/of_regulator.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
> index cbc1d71..cd65885 100644
> --- a/drivers/regulator/of_regulator.c
> +++ b/drivers/regulator/of_regulator.c
> @@ -25,7 +25,8 @@ static const char *const regulator_states[PM_SUSPEND_MAX + 1] = {
>  };
>  
>  static void of_get_regulation_constraints(struct device_node *np,
> -					struct regulator_init_data **init_data)
> +					struct regulator_init_data **init_data,
> +					const struct regulator_desc *desc)
>  {
>  	const __be32 *min_uV, *max_uV;
>  	struct regulation_constraints *constraints = &(*init_data)->constraints;
> @@ -81,6 +82,14 @@ static void of_get_regulation_constraints(struct device_node *np,
>  	if (!ret)
>  		constraints->enable_time = pval;
>  
> +	if (!of_property_read_u32(np, "regulator-initial-mode", &pval)) {
> +		if (desc && desc->map_modes)
> +			constraints->initial_mode = desc->map_modes(pval);
> +		else
> +			pr_warn("%s: failed to parse regulator-initial-mode\n",
> +				np->name);
> +	}
> +

Here's a hidden assumption that if driver does not provide map_modes
then any "regulator-initial-mode" property is not valid. Shouldn't this
be mentioned somewhere? Maybe in description of map_modes callback?

Best regards,
Krzysztof


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

Re: [PATCH v4 00/14] Add max77802 regulator operating mode support

[Krzysztof Kozlowski]

On pon, 2014-11-03 at 15:40 +0100, Javier Martinez Canillas wrote:
> Hello Mark,
> 
> This is the fourth version of the series that adds operating modes
> support for the regulators in the max77802 PMIC. This version uses
> the standard suspend states bindings and the opmodes are parsed by
> the regulator core while drivers only define a translation function
> to map between hardware specific to standard modes as you suggested.
> 
> The series adds a "regulator-initial-mode" property to configure at
> startup the operating mode for the regulators that support changing
> its mode during normal operation and a "regulator-mode" property for
> the regulators that supports changing its operating mode when the
> system enters in a suspend state. These properties were originally
> part of Chanwoo Choi's regulator suspend state series [0] but were
> removed since there wasn't a way to define the operating modes in a
> generic way.
> 
> In this series, the generic regulator DT binding doc explains that each
> device has to document what their valid operating modes are. Drivers
> must provide a translation function so the core can map the modes.
> 
> Since parsing the modes in the core is a very different approach, most
> of the patches are new but those that remains have a changelog.
> 
> This series depend on [0] and also v2 of patch:
> "ARM: EXYNOS: Call regulator core suspend prepare and finish functions" [1].
> 
> Javier Martinez Canillas (14):
>   regulator: Document binding for initial and suspend modes
>   regulator: Add function to map modes to struct regulator_desc
>   regulator: of: Add regulator desc param to
>     of_get_regulator_init_data()
>   regulator: of: Pass the regulator description in the match table
>   regulator: max1586: zero-initialize regulator match table array
>   regulator: max77686: zero-initialize regulator match table
>   regulator: max77802: zero-initialize regulator match table
>   regulator: max8660: zero-initialize regulator match table array
>   regulator: s2mpa01: zero-initialize regulator match table array
>   regulator: of: Add support for parsing initial and suspend modes
>   regulator: max77802: Document binding for regulator operating modes
>   regulator: max77802: Use unsigned int for modes in max77802_map_mode()
>   regulator: max77802: Set regulator modes translation callback
>   ARM: dts: Configure regulators for suspend on exynos Peach boards
> 
> Patch #1 extends the regulator DT binding to document the initial and
> suspend modes properties.
> 
> Patch #2 adds a function pointer to the static regulator description
> so drivers can define a callback that does the modes translation.
> 
> Patch #3 does some refactoring to pass the regulator descriptor to the
> function extracting the regulator initial data from DT.
> 
> Patch #4 adds a pointer to the regulator descriptor in the match table
> so users extracting the init_data from of_regulator_match can also map
> the modes.
> 
> Patch #5-#9 are fixes to be sure that all callers are passing an
> initialised match table.
> 
> Patch #10 modifies the function that extracts the regulator data from
> DT to parse the initial and suspend modes.
> 
> Patch #11 extends the max77802 DT binding to document the valid opmodes
> for the regulators on this device.
> 
> Patch #12 change the signature of the function doing the modes mapping
> for the max77802 regulators mode.
> 
> Patch #13 set the function handler for the max77802 modes translation.
> 
> Patch #14 configure the regulators for the Peach Pit and Pi Chromebooks.
> 
> Best regards,
> Javier
> 
> [0]: https://lkml.org/lkml/2014/10/10/161
> [1]: http://www.spinics.net/lists/arm-kernel/msg369923.html

Where's a diff stat? It is helpful to see what files were touched and I
believe it is created by default with format-patch.

Bet regards,
Krzysztof

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

Re: [PATCH v4 11/14] regulator: max77802: Document binding for regulator operating modes

[Krzysztof Kozlowski]

On pon, 2014-11-03 at 15:40 +0100, Javier Martinez Canillas wrote:
> Some regulators from the max77802 PMIC support to be configured in one
> of two operating mode: Output ON (normal) and Output On Low Power Mode.
> Not all regulators support these two modes and for some of them, the
> mode can be changed while the system is running in normal operation
> while others only support their mode to be changed on system suspend.
> 
> Extend the max77802 PMIC bindin by documenting the possible operating
> modes values so the regulators modes can be correctly configured.
> 
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
> 
> Changes in v4: None
> 
> Changes in v3:
>  - Use the standard suspend states bindings as suggested by Mark Brown.
> 
>  .../devicetree/bindings/regulator/max77802.txt     | 33 ++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/regulator/max77802.txt b/Documentation/devicetree/bindings/regulator/max77802.txt
> index 5aeaffc..1a78ec2 100644
> --- a/Documentation/devicetree/bindings/regulator/max77802.txt
> +++ b/Documentation/devicetree/bindings/regulator/max77802.txt
> @@ -25,6 +25,27 @@ with their hardware counterparts as follow. The valid names are:
>  			example: LDO1, LDO2, LDO35.
>  	-BUCKn 	:	for BUCKs, where n can lie in range 1 to 10.
>  			example: BUCK1, BUCK5, BUCK10.
> +
> +The max77802 regulator supports two different operating modes: Normal and Low
> +Power Mode. Some regulators support the modes to be changed at startup or by
> +the consumers during normal operation while others only support to change the
> +mode during system suspend. The standard regulator suspend states binding can
> +be used to configure the regulator operating mode.
> +
> +The regulators that support the standard "regulator-initial-mode" property,
> +changing their mode during normal operation are: LDOs 1, 3, 20 and 21.
> +
> +The possible values for "regulator-initial-mode" and "regulator-mode" are:
> +	1: Normal regulator voltage output mode.
> +	3: Low Power which reduces the quiescent current down to only 1uA
> +
> +The list of valid modes are defined in the dt-bindings/regulator/regulator.h
> +header and can be included by device tree source files.

The more he looked inside, the more regulator.h wasn't there.

Best regards,
Krzysztof

Re: [PATCH v4 00/14] Add max77802 regulator operating mode support

[Javier Martinez Canillas]

Hello Krzysztof,

On 11/04/2014 11:44 AM, Krzysztof Kozlowski wrote:
> 
> Where's a diff stat? It is helpful to see what files were touched and I
> believe it is created by default with format-patch.
>

Yes, I removed the diffstat since it was quite big due the drivers
refactor and the cover letter was already long but I'll add it in
the next revision.
 
> Bet regards,
> Krzysztof
> 

Best regards,
Javier

Re: [PATCH v4 02/14] regulator: Add function to map modes to struct regulator_desc

[Javier Martinez Canillas]

Hello Krzysztof,

Thanks a for your feedback.

On 11/04/2014 11:31 AM, Krzysztof Kozlowski wrote:
>> + *
>> + * @map_modes: Callback invoked to translate between hardware to standard modes.
> 
> Initially I thought it should map from standard to hardware. But then I
> looked at max77802 implementation and it maps from hardware to standard.
> Anyway I got confused (both are "modes" and both unsigned ints).
> 
> Could you describe which should be returned?
> 

Sure, maybe rewording to:

"Callback invoked to translate from hardware to standard modes." ?

But I'll add also document that the parameter should be a hardware
mode and the return value a standard mode.

 >>   */
>>  struct regulator_desc {
>>  	const char *name;
>> @@ -285,6 +287,8 @@ struct regulator_desc {
>>  	unsigned int enable_time;
>>  
>>  	unsigned int off_on_delay;
>> +
>> +	unsigned int (*map_modes)(unsigned int mode);
> 
> Shouldn't this be in regulator ops?
> 

regulator ops are for the operations that a regulator support
(enable, disable, set mode, etc). All the thse are actions but
how to translate between hardware and standard modes is not an
action but a non-varying configuration of the regulator.

So I believe that regulator desc was what fit the most. I don't
have a strong opinion though if people think that it should be
in regulator ops instead.

Best regards,
Javier

Re: [PATCH v4 00/14] Add max77802 regulator operating mode support

[Krzysztof Kozlowski]

On wto, 2014-11-04 at 11:51 +0100, Javier Martinez Canillas wrote:
> Hello Krzysztof,
> 
> On 11/04/2014 11:44 AM, Krzysztof Kozlowski wrote:
> > 
> > Where's a diff stat? It is helpful to see what files were touched and I
> > believe it is created by default with format-patch.
> >
> 
> Yes, I removed the diffstat since it was quite big due the drivers
> refactor and the cover letter was already long but I'll add it in
> the next revision.

I was searching for regulator.h and I hope to find it in diffstat...

Best regards,
Krzysztof

Re: [PATCH v4 10/14] regulator: of: Add support for parsing initial and suspend modes

[Javier Martinez Canillas]

Hello Krzysztof,

On 11/04/2014 11:41 AM, Krzysztof Kozlowski wrote:
>> +	if (!of_property_read_u32(np, "regulator-initial-mode", &pval)) {
>> +		if (desc && desc->map_modes)
>> +			constraints->initial_mode = desc->map_modes(pval);
>> +		else
>> +			pr_warn("%s: failed to parse regulator-initial-mode\n",
>> +				np->name);
>> +	}
>> +
> 
> Here's a hidden assumption that if driver does not provide map_modes
> then any "regulator-initial-mode" property is not valid. Shouldn't this
> be mentioned somewhere? Maybe in description of map_modes callback?
> 

Well it can't be valid if the regulator core does not know how to map the
value in the property to one of the standard modes. The binding explains
that each PMIC has to define its hardware modes, I can also add a note
in the .map_modes. I guess no one will say no to more documentation :-)

Best regards,
Javier

Re: [PATCH v4 02/14] regulator: Add function to map modes to struct regulator_desc

[Krzysztof Kozlowski]

On wto, 2014-11-04 at 12:02 +0100, Javier Martinez Canillas wrote:
> Hello Krzysztof,
> 
> Thanks a for your feedback.
> 
> On 11/04/2014 11:31 AM, Krzysztof Kozlowski wrote:
> >> + *
> >> + * @map_modes: Callback invoked to translate between hardware to standard modes.
> > 
> > Initially I thought it should map from standard to hardware. But then I
> > looked at max77802 implementation and it maps from hardware to standard.
> > Anyway I got confused (both are "modes" and both unsigned ints).
> > 
> > Could you describe which should be returned?
> > 
> 
> Sure, maybe rewording to:
> 
> "Callback invoked to translate from hardware to standard modes." ?
> 
> But I'll add also document that the parameter should be a hardware
> mode and the return value a standard mode.

Great!

> 
>  >>   */
> >>  struct regulator_desc {
> >>  	const char *name;
> >> @@ -285,6 +287,8 @@ struct regulator_desc {
> >>  	unsigned int enable_time;
> >>  
> >>  	unsigned int off_on_delay;
> >> +
> >> +	unsigned int (*map_modes)(unsigned int mode);
> > 
> > Shouldn't this be in regulator ops?
> > 
> 
> regulator ops are for the operations that a regulator support
> (enable, disable, set mode, etc). All the thse are actions but
> how to translate between hardware and standard modes is not an
> action but a non-varying configuration of the regulator.
> 
> So I believe that regulator desc was what fit the most. I don't
> have a strong opinion though if people think that it should be
> in regulator ops instead.

I understand, it's fine for me.

Best regards,
Krzysztof

Re: [PATCH v4 11/14] regulator: max77802: Document binding for regulator operating modes

[Javier Martinez Canillas]

Hello Krzysztof,

On 11/04/2014 11:50 AM, Krzysztof Kozlowski wrote:
>> +
>> +The possible values for "regulator-initial-mode" and "regulator-mode" are:
>> +	1: Normal regulator voltage output mode.
>> +	3: Low Power which reduces the quiescent current down to only 1uA
>> +
>> +The list of valid modes are defined in the dt-bindings/regulator/regulator.h
>> +header and can be included by device tree source files.
> 
> The more he looked inside, the more regulator.h wasn't there.
> 

Right, that's a typo from when the documentation was updated since previous
versions used the standard modes instead of hardware specific modes.

Should say that are defined in include/dt-bindings/regulator/maxim,max77802.h
instead, sorry about that.

Best regards,
Javier